[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 12/18] qemu: Support memballoon model=virtio-{non-}transitional



On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
[...]
> +++ b/docs/formatdomain.html.in
> @@ -7902,6 +7902,8 @@ qemu-kvm -net nic,model=? /dev/null
>          </p>
>          <ul>
>            <li>'virtio' - default with QEMU/KVM</li>
> +          <li>'virtio-transitional'</li>
> +          <li>'virtio-non-transitional'</li>
>            <li>'xen' - default with Xen</li>
>          </ul>

You didn't add the "Since: 5.1.0" here.

[...]
> +++ b/docs/schemas/domaincommon.rng
> @@ -4091,6 +4091,8 @@
>            <value>virtio</value>
>            <value>xen</value>
>            <value>none</value>
> +          <value>virtio-transitional</value>
> +          <value>virtio-non-transitional</value>
>          </choice>

I'd sort these new <value>s right after "virtio".

[...]
> @@ -550,7 +550,9 @@ VIR_ENUM_IMPL(virDomainKeyWrapCipherName,
>  VIR_ENUM_IMPL(virDomainMemballoonModel, VIR_DOMAIN_MEMBALLOON_MODEL_LAST,
>                "virtio",
>                "xen",
> -              "none")
> +              "none",
> +              "virtio-transitional",
> +              "virtio-non-transitional")

Same comment as for other VIR_ENUM_IMPL() calls, and also I'd sort
the values as described above.

[...]
> @@ -1132,6 +1134,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
>      {"virtio-rng-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_RNG_NON_TRANSITIONAL},
>      {"virtio-9p-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_9P_TRANSITIONAL},
>      {"virtio-9p-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_9P_NON_TRANSITIONAL},
> +    {"virtio-balloon-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_BALLOON_TRANSITIONAL},
> +    {"virtio-balloon-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BALLOON_NON_TRANSITIONAL},
>  };

Usual comment for capabilities.

[...]
> @@ -2284,8 +2287,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
>      }
>  
>      /* VirtIO balloon */
> -    if (def->memballoon &&
> -        def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO &&
> +    if (virDomainDefHasMemballoon(def) &&
>          virDeviceInfoPCIAddressIsWanted(&def->memballoon->info)) {

See comments on the previous patch for why I don't think this is
correct; either way, it should have been part of *that* patch, no?

-- 
Andrea Bolognani / Red Hat / Virtualization


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]