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

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

 




On 27.02.2019 20:43, John Ferlan wrote:
> 
> 
> 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...

Thanx, great hint!

> 
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
>> ---
>>  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.

AFAIU tristate is nice when we have hypervisor-specific default values.
In this case we not. So tristate won't be that useful. So tristate
is implementation matter decoupled from RNG.

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

I think we should print "no" for PCI devices if @managed is omitted on input
as we do usually after expanding default values.


Nikolay

> 
> 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)
> 
> [...]
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux