[libvirt] [PATCH v3 01/12] conf: Add definitions for 'uid' and 'fid' PCI address attributes

[Andrea Bolognani]

On Thu, 2018-08-23 at 14:40 +0800, Yi Min Zhao wrote:
> 在 2018/8/22 下午6:09, Andrea Bolognani 写道:
> > On Wed, 2018-08-22 at 17:39 +0800, Yi Min Zhao wrote:
> > > I tried as your idea. It makes everything complicated, especially
> > > alloc/reserve/release
> > > zpci address. If the user defines uid=1 and fid=0, we don't know whether
> > > should
> > > reserve fid. (uid=1 fid=0) is the same with (uid=1).
> > 
> > You should reserve it. The user can either
> > 
> >    * not specify zPCI information at all: automatic assignment will
> >      be performed, no error should be reported;
> > 
> >    * specify a *full* zPCI address: manual assignment, will result
> >      in either that exact address being used or an error;
> > 
> >    * specify partial information: missing properties will default
> >      to zero, which will probably cause errors eg. if only the uid
> >      is specified for a bunch of devices.
> > 
> > The last option is less predictable so it should probably never be
> > used. All of the above is consistent with how the PCI part works.
> 
> And then, zpci address instance could be a member of pci address structure?
> I mean not using pointer at all?

Exactly. You can either just add zpci_uid and zpci_fid to the
virPCIDeviceAddress struct, or have

  struct _virZPCIDeviceAddress {
      unsigned int uid;
      unsigned int fid;
  };

  struct _virPCIDeviceAddress {
      unsigned int domain;
      unsigned int bus;
      unsigned int slot;
      unsigned int function;
      int multi; /* virTristateSwitch */
      virZPCIDeviceAddress zpci;
  };

that is, have a separate struct for zPCI but *embed it* inside the
existing one instead of allocating it separately, the same way we
embed virPCIDeviceAddress itself virDomainDeviceInfo, for example.
This second option looks better to me.

This got me thinking that the extension flags *also* belongs to
virPCIDeviceAddress, since we need them to know whether the zPCI
part should be taken into account when formatting and such: you
used to check whether the zpci pointer was NULL, but of course you
can no longer do that once the pointer is gone; moreover, checking
for a flag is more explicit so that's also an improvement. Using
the flags stored into virDomainDeviceInfo would be ugly because it
would make it so virPCIDeviceAddress is no longer a stand-alone
object, so we should avoid that.

I'm not sure you can get away with not storing the extension flags
in virDomainDeviceInfo, though, because IIRC you might need to use
them *before* you have figured out that you're going to assign a
PCI address to the device. We might just have to store them twice,
which is not great but I think preferable to introducing a reverse
dependency from virPCIDeviceAddress to virDomainDeviceInfo. But I
haven't really looked too closely into it, so perhaps there's a
way to avoid that duplication after all :)

-- 
Andrea Bolognani / Red Hat / Virtualization