
On Thu, 2016-08-11 at 13:57 +0200, Ján Tomko wrote:
Check whether the disable-legacy property is present on the following devices: virtio-balloon-pci virtio-blk-pci virtio-scsi-pci virtio-serial-pci virtio-9p-pci virtio-net-pci virtio-rng-pci virtio-gpu-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci Assuming that if QEMU knows other virtio devices where this property is applicable, it will have at least one of these devices. Added in QEMU by: commit e266d421490e0ae83044bbebb209b2d3650c0ba6 virtio-pci: add flags to enable/disable legacy/modern
I looked at this patch because it's a requirement for Laine's PCIe series. I'll just point out a couple things; I see there have been a few comments about the design of the interface that you'll need to address, so I don't think it's very useful to look at the whole series before you've had a chance to do so.
+struct virQEMUCapsPropObjects { + const char *prop; + int flag; + const char **objects; +}; + +static const char *virQEMUCapsVirtioPCIDisableLegacyObjects[] = { + "virtio-balloon-pci", + "virtio-blk-pci", + "virtio-scsi-pci", + "virtio-serial-pci", + "virtio-9p-pci", + "virtio-net-pci", + "virtio-rng-pci", + "virtio-gpu-pci", + "virtio-input-host-pci", + "virtio-keyboard-pci", + "virtio-mouse-pci", + "virtio-tablet-pci", + NULL +}; + +static struct virQEMUCapsPropObjects virQEMUCapsPropObjects[] = {
Please, don't :) Use something like virQEMUCapsPropTypeObjects (to mirror the existing virQEMUCapsObjectTypeProps), or virQEMUCapsPropObjectsType, or anything really - just make sure the name of the type and the name of the variable containing a bunch of instances of said type are not the same.
static void +virQEMUCapsProcessProps(virQEMUCapsPtr qemuCaps, + size_t nprops, + struct virQEMUCapsPropObjects *props, + const char *object, + size_t nvalues, + char *const*values) +{ + size_t i, j; + + for (i = 0; i < nprops; i++) { + if (virQEMUCapsGet(qemuCaps, props[i].flag)) + continue; + + for (j = 0; j < nvalues; j++) { + if (STREQ(values[j], props[i].prop)) { + if (virStringArrayHasString((char **)props[i].objects, object))
Rather than casting a const char ** to char **, which happens in other places as well, it would be IMHO much better to make virStringArrayHasString() accept a const char ** as the first argument. And guess what? I just posted a patch[1] that does exactly that :) Everything else looks good. [1] https://www.redhat.com/archives/libvir-list/2016-August/msg00784.html -- Andrea Bolognani / Red Hat / Virtualization