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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list