[PATCH v1 05/26] qemu_domain_address: Reformat qemuDomainAssignS390Addresses()

[Michal Privoznik]

On 11/30/20 10:38 AM, Thomas Huth wrote:
> On 27/11/2020 16.02, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>   src/qemu/qemu_domain_address.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
>> index 2788dc7fb3..d872f75b38 100644
>> --- a/src/qemu/qemu_domain_address.c
>> +++ b/src/qemu/qemu_domain_address.c
>> @@ -408,18 +408,16 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def,
>>       if (qemuDomainIsS390CCW(def) &&
>>           virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCW)) {
>>           if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW))
>> -            qemuDomainPrimeVfioDeviceAddresses(
>> -                def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
>> -        qemuDomainPrimeVirtioDeviceAddresses(
>> -            def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
>> +            qemuDomainPrimeVfioDeviceAddresses(def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
> 
> Looks fine to me, but docs/coding-style.rst still suggest to format code
> with "indent -l75", so is this really the right thing to do here?

It's true that we have 80 characters limit, but that is more of a soft 
limit and common sense should be used. Personally, I find

    func(
arg1, arg2
);

worse than exceeding that 80 char rule. My common sense tells me that 
the rule tries to avoid the following pattern (among others):

   func(arg1, arg2, ...., very_long_list_of_arguments, which, could, 
easily, go_on_multiple_lines, but, didnt);


> 
>> +        qemuDomainPrimeVirtioDeviceAddresses(def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
>>   
>>           if (!(addrs = virDomainCCWAddressSetCreateFromDomain(def)))
>>               goto cleanup;
>>   
>>       } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
> 
> Not related to your patch, but an idea for a future clean-up: That
> QEMU_CAPS_VIRTIO_S390 seems to belong to the ancient "s390-virtio" (without
> ccw) machine that has been removed in QEMU v2.6 already:
> 
> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=7b3fdbd9a82
> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=3538fb6f89d
> 
> IIRC, that machine was already considered as deprecated since a couple of
> earlier QEMU releases, so I really doubt that anybody is still using that in
> production today.
> 
> Thus I think that all code related to QEMU_CAPS_VIRTIO_S390 could likely be
> removed from libvirt nowadays.

That is even better idea. But currently libvirt supports QEMU-1.5.0 and 
newer. So I think we shouldn't remove that until the minimum version is 
bumped even though we think feature has no users.

https://gitlab.com/libvirt/libvirt/-/commit/c1bc9c662b4

Although, it might be about time to look again what is the oldest QEMU 
we need to support.

Michal