[libvirt] [PATCH 08/18] qemu: Support rng model=virtio-{non-}transitional

[Andrea Bolognani]

On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
[...]
>  VIR_ENUM_IMPL(virDomainRNGModel,
>                VIR_DOMAIN_RNG_MODEL_LAST,
> -              "virtio");
> +              "virtio",
> +              "virtio-transitional",
> +              "virtio-non-transitional");

Since you're touching the definition, you can make it

  VIR_ENUM_IMPL(virDomainRNGModel,
                ...
                "virtio-non-transitional",
  );

so that if and when we'll need to add another model the diff
will look nicer.

[...]
> @@ -1122,6 +1124,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>      {"virtio-net-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL},
>      {"vhost-scsi-pci-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL},
>      {"vhost-scsi-pci-non-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL},
> +    {"virtio-rng-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_RNG_TRANSITIONAL},
> +    {"virtio-rng-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_RNG_NON_TRANSITIONAL},
>  };

Same comments as for the other capabilities.

[...]
> @@ -5990,7 +5995,7 @@ qemuBuildRNGDevStr(const virDomainDef *def,
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
>  
> -    if (dev->model != VIR_DOMAIN_RNG_MODEL_VIRTIO ||
> +    if (dev->model == VIR_DOMAIN_RNG_MODEL_VIRTIO &&
>          !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_RNG)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("this qemu doesn't support RNG device type '%s'"),

Idea for a possible follow-up series: wouldn't it be great if we
centralized capabilities checking for VirtIO devices in a function
that can be called during device validation? Delaying such checks
until command line generation is not optimal, and neither is having
some of them centralized and some of them sprinkled around the
various qemuBuild*DevStr() functions...

[...]
> @@ -2290,8 +2295,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
>  
>      /* VirtIO RNG */
>      for (i = 0; i < def->nrngs; i++) {
> -        if (def->rngs[i]->model != VIR_DOMAIN_RNG_MODEL_VIRTIO ||
> -            !virDeviceInfoPCIAddressIsWanted(&def->rngs[i]->info))
> +        if (!virDeviceInfoPCIAddressIsWanted(&def->rngs[i]->info))
>              continue;

I'm not convinced you can just remove the model check here...
Shouldn't you extend it to include MODEL_VIRTIO_TRANSITIONAL and
MODEL_VIRTIO_NON_TRANSITIONAL instead?

-- 
Andrea Bolognani / Red Hat / Virtualization