[libvirt] [PATCH v4 2/2] qemu_capabilities: Introduce gluster specific debug capability

[Prasanna Kalever]

On Wed, Sep 21, 2016 at 7:21 PM, Jiri Denemark <jdenemar at redhat.com> wrote:
> On Wed, Sep 21, 2016 at 19:00:17 +0530, Prasanna Kumar Kalever wrote:
>> Teach qemu driver to detect whether qemu supports this configuration
>> factor or not.
>>
>> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever at redhat.com>
> ...i
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 545e25d..b73ad70 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1314,7 +1314,8 @@ qemuDiskBusNeedsDeviceArg(int bus)
>>
>>  static int
>>  qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
>> -                        virBufferPtr buf)
>> +                        virBufferPtr buf,
>> +                        virQEMUCapsPtr qemuCaps)
>>  {
>>      int actualType = virStorageSourceGetActualType(disk->src);
>>      qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>> @@ -1385,7 +1386,8 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
>>
>>      if (disk->src &&
>>          disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
>> -        virBufferAsprintf(buf, "file.debug=%d,", disk->src->debug_level);
>> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL))
>> +            virBufferAsprintf(buf, "file.debug=%d,", disk->src->debug_level);
>
> Adding file.debug unconditionally first and making it conditional in the
> next patch is pretty strange. The capability should really be introduced
> as the first patch in the series to avoid it.

This is something I have learned form the log/history.
As long as these are in series, I don't think that really matters ?
Also the cover letter prepares the reviewers for it, do let me know if
it is unclear there.

In case, if you think this is blocker/stopping this patch, let me know
I shall squash them up.

Thanks Jirka!
--
Prasanna

>
>>      }
>>
>>      if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
>> @@ -1513,7 +1515,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>>          break;
>>      }
>>
>> -    if (qemuBuildDriveSourceStr(disk, &opt) < 0)
>> +    if (qemuBuildDriveSourceStr(disk, &opt, qemuCaps) < 0)
>>          goto error;
>>
>>      if (emitDeviceSyntax)
>> @@ -2263,8 +2265,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>>
>>          if (disk->src &&
>>              disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
>> -            if (cfg->glusterDebugLevel)
>> -                disk->src->debug_level = cfg->glusterDebugLevel;
>> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL) &&
>> +                cfg->glusterDebugLevel)
>> +                    disk->src->debug_level = cfg->glusterDebugLevel;
>
> Wrong indentation, but it doesn't matter much because of my comment on
> this code in patch 1.
> ...
>
> Jirka