[libvirt] [PATCH] conf: don't output managed attr for non-pci hostdevs

[John Ferlan]

On 2/22/19 4:35 AM, Nikolay Shirokovskiy wrote:
> It is ignored on input for non pci hostdevs as written in docs.
> I guess it would be better if the attr was disabled to be specified
> in the first place instead but such behaviour is already documented.
> At least let's not output it back. This will fix issue when
> managed appeared on output for non pci hostdevs when it was not
> specified on input.
> 
> The tests are fixed by next commands:
> grep  -lRP 'hostdev.*mode=.subsystem.*type=.(?!pci).*managed' `find tests -name '*.xml'` > files
> sed -i.bak -re 's/ managed=.(yes|no).//' `cat files`
> 

To save yourself some effort in the future...

VIR_TEST_REGENERATE_OUTPUT=1 tests/qemuxml2xmltest
VIR_TEST_REGENERATE_OUTPUT=1 tests/xlconfigtest
VIR_TEST_REGENERATE_OUTPUT=1 tests/qemuargv2xmltest

Would have got a lot of them...

> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/conf/domain_conf.c                             |  6 ++++--
>  tests/qemuargv2xmldata/hostdev-usb-address.xml     |  2 +-
>  tests/qemuxml2argvdata/controller-order.xml        |  2 +-
>  .../disk-hostdev-scsi-address-conflict.xml         |  2 +-
>  .../disk-hostdev-scsi-virtio-iscsi-auth-AES.xml    |  2 +-
>  .../hostdev-scsi-autogen-address.xml               | 22 +++++++++++-----------
>  tests/qemuxml2argvdata/hostdev-scsi-boot.xml       |  2 +-
>  tests/qemuxml2argvdata/hostdev-scsi-large-unit.xml |  2 +-
>  .../hostdev-scsi-lsi-iscsi-auth.xml                |  4 ++--
>  tests/qemuxml2argvdata/hostdev-scsi-lsi-iscsi.xml  |  4 ++--
>  tests/qemuxml2argvdata/hostdev-scsi-lsi.xml        |  2 +-
>  tests/qemuxml2argvdata/hostdev-scsi-rawio.xml      |  2 +-
>  tests/qemuxml2argvdata/hostdev-scsi-readonly.xml   |  2 +-
>  tests/qemuxml2argvdata/hostdev-scsi-sgio.xml       |  2 +-
>  tests/qemuxml2argvdata/hostdev-scsi-shareable.xml  |  2 +-
>  .../hostdev-scsi-vhost-scsi-ccw.xml                |  2 +-
>  .../hostdev-scsi-vhost-scsi-pci.xml                |  2 +-
>  .../hostdev-scsi-vhost-scsi-pcie.xml               |  2 +-
>  .../hostdev-scsi-virtio-iscsi-auth.xml             |  4 ++--
>  .../qemuxml2argvdata/hostdev-scsi-virtio-iscsi.xml |  4 ++--
>  .../qemuxml2argvdata/hostdev-scsi-virtio-scsi.xml  |  2 +-
>  .../hostdev-usb-address-device-boot.xml            |  2 +-
>  .../hostdev-usb-address-device.xml                 |  2 +-
>  tests/qemuxml2argvdata/hostdev-usb-address.xml     |  2 +-
>  .../hostdevs-drive-address-conflict.xml            |  4 ++--
>  tests/qemuxml2argvdata/name-escape.xml             |  2 +-
>  tests/qemuxml2argvdata/user-aliases-usb.xml        |  4 ++--
>  tests/qemuxml2xmloutdata/hostdev-mdev-display.xml  |  2 +-
>  .../qemuxml2xmloutdata/hostdev-mdev-precreated.xml |  2 +-
>  .../hostdev-scsi-autogen-address.xml               | 22 +++++++++++-----------
>  .../qemuxml2xmloutdata/hostdev-scsi-large-unit.xml |  2 +-
>  .../hostdev-scsi-lsi-iscsi-auth.xml                |  4 ++--
>  .../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml  |  4 ++--
>  tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml      |  2 +-
>  tests/qemuxml2xmloutdata/hostdev-scsi-rawio.xml    |  2 +-
>  tests/qemuxml2xmloutdata/hostdev-scsi-readonly.xml |  2 +-
>  tests/qemuxml2xmloutdata/hostdev-scsi-sgio.xml     |  2 +-
>  .../qemuxml2xmloutdata/hostdev-scsi-shareable.xml  |  2 +-
>  .../hostdev-scsi-vhost-scsi-ccw.xml                |  2 +-
>  .../hostdev-scsi-vhost-scsi-pci.xml                |  2 +-
>  .../hostdev-scsi-vhost-scsi-pcie.xml               |  2 +-
>  .../hostdev-scsi-virtio-iscsi-auth.xml             |  4 ++--
>  .../hostdev-scsi-virtio-iscsi.xml                  |  4 ++--
>  .../hostdev-scsi-virtio-scsi.xml                   |  2 +-
>  .../hostdev-subsys-mdev-vfio-ccw.xml               |  2 +-
>  tests/qemuxml2xmloutdata/hostdev-usb-address.xml   |  2 +-
>  tests/xlconfigdata/test-usb.xml                    |  2 +-
>  47 files changed, 80 insertions(+), 78 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ceeb247..9ff659c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -27330,8 +27330,10 @@ virDomainHostdevDefFormat(virBufferPtr buf,
>      virBufferAsprintf(buf, "<hostdev mode='%s' type='%s'",
>                        mode, type);
>      if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> -        virBufferAsprintf(buf, " managed='%s'",
> -                          def->managed ? "yes" : "no");
> +        if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
> +            virBufferAsprintf(buf, " managed='%s'",
> +                              def->managed ? "yes" : "no");
> +        }

Do you think perhaps @managed should be changed to a virTristateBool?
After all the RNG attribute is a virYesNo like other Tristate's.

That way the printing would only be done *if* found specifically on input.

The problem with blindly removing the "no" even though it's listed as
not being parsed on input is the differences it creates in output when
someone for some reason does add "managed='{yes|no}'" and now we
"remove" that...

So, I'm conflicted on the best resolution for this.  It is just a test
and PCI is listed as the only one that would manage it, but it is also
said the others would ignore it other than of course generating the
output blindly which you're trying to change/fix.

Perhaps there's some other opinions on this.

John

>  
>          if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
>              scsisrc->sgio)

[...]