On Wed, Sep 21, 2016 at 7:28 PM, Jiri Denemark <jdenemar@xxxxxxxxxx> wrote:
> On Wed, Sep 21, 2016 at 19:00:16 +0530, Prasanna Kumar Kalever wrote:
>> This helps in selecting log level of the gluster gfapi, output to stderr.
>> The option is 'gluster_debug_level', can be tuned by editing
>> '/etc/libvirt/qemu.conf'
>>
>> Debug levels ranges 0-9, with 9 being the most verbose, and 0 representing
>> no debugging output. The default is the same as it was before, which
>> is a level of 4. The current logging levels defined in the gluster
>> gfapi are:
>>
>> 0 - None
>> 1 - Emergency
>> 2 - Alert
>> 3 - Critical
>> 4 - Error
>> 5 - Warning
>> 6 - Notice
>> 7 - Info
>> 8 - Debug
>> 9 - Trace
>>
>> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@xxxxxxxxxx>
> ...
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 3a61863..545e25d 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1383,6 +1383,11 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
>> }
>> virBufferAddLit(buf, ",");
>>
>> + if (disk->src &&
>> + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
>> + virBufferAsprintf(buf, "file.debug=%d,", disk->src->debug_level);
>
> This should be only done if QEMU supports it; see my comment to patch 2.
Addressed in patch 2/2
>
> And just use cfg->glusterDebugLevel directly here instead of passing it
> via disk->src->debug_level.
Just felt it will be too much routing of cfg to all the way in the
function stack just for the sake of glusterDebugLevel.
Hence I went with storing it in storage structure.
>
>> + }
>> +
>> if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
>> /* NB: If libvirt starts using the more modern option based
>> * syntax to build the command line (e.g., "-drive driver=rbd,
>> @@ -2177,6 +2182,7 @@ qemuBuildDriveDevStr(const virDomainDef *def,
>>
>> static int
>> qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>> + virQEMUDriverConfigPtr cfg,
>> const virDomainDef *def,
>> virQEMUCapsPtr qemuCaps)
>> {
>> @@ -2255,6 +2261,12 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>>
>> virCommandAddArg(cmd, "-drive");
>>
>> + if (disk->src &&
>> + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
>> + if (cfg->glusterDebugLevel)
>> + disk->src->debug_level = cfg->glusterDebugLevel;
>> + }
>> +
>> if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps)))
>> return -1;
>> virCommandAddArg(cmd, optstr);
>
> Please, don't store the static debug level in disk->src. NACK to this
> hunk.
Since Peter also pointed about the same in the initial patches, let me
pass cfg to all the callee's. Never Mind.
>
> ...
>> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
>> index 3d09468..9f3add3 100644
>> --- a/src/util/virstoragefile.h
>> +++ b/src/util/virstoragefile.h
>> @@ -237,6 +237,8 @@ struct _virStorageSource {
>> virStorageAuthDefPtr auth;
>> virStorageEncryptionPtr encryption;
>>
>> + unsigned int debug_level;
>> +
>> char *driverName;
>> int format; /* virStorageFileFormat in domain backing chains, but
>> * pool-specific enum for storage volumes */
>
> And NACK to this hunk, too.
taken care when using/passing cfg.
--
Prasanna
> ...
>
> Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list