Re: [libvirt] [PATCH v2 RESEND 09/12] qemu: Generate and use zPCI device in QEMU command line
On Fri, 2018-07-27 at 15:53 +0800, Yi Min Zhao wrote:
在 2018/7/24 下午11:09, Andrea Bolognani 写道:
> Also, do you really need to check both the flags and the presence
> of the zPCI address bits? It looks like either one or the other
> should be enough or, if that's not the case, it should be made so
> because having to check for two separate conditions makes me feel
> like it would introduce bugs in the long run.
This is actually a problem. I add info->addr.pci.zpci check in order to
check
if the user specifies zpci address in xml even though it has no zpci
support.
The callers of this function checks zpci capability. If no zpci cap but
the user
specfies zpci address, report an error.
I will change the logic and remove the check on info->addr.pci.zpci.
Yeah, you definitely want to report an error if the QEMU binary
doesn't support zPCI or the user is attempting to do something
silly like add zPCI-related information to devices attached to
an x86_64 guest...
> > +char *qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev);
> > +
> > +bool qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info);
>
> Is this really necessary? Can't these two functions be static?
They are also used in qemu_hotplug.c file.
Right, I overlooked that :)
That said, they aren't used in the hotplug code until the next
patch, so it would IMHO make sense to define them as static in
this patch and export them in the next one, at the same time as
you actually start using them outside of the file.
[...]
> > + DO_TEST("hostdev-vfio-zpci",
> > + QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI);
> > + DO_TEST("hostdev-vfio-zpci-multidomain-many",
> > + QEMU_CAPS_DEVICE_VFIO_PCI, X_QEMU_CAPS_HOST_PCI_MULTIDOMAIN,
>
> Capabilities with X_QEMU prefix are no longer used, so you should
> not list them here.
Sure.
I forgot to mention, please have a single capability per line and
make sure the set of capabilities used in xml2xml and xml2argv is
exactly the same.
--
Andrea Bolognani / Red Hat / Virtualization