Re: [libvirt] [PATCH v4 1/2] qemu/gluster: add option for tuning debug logging level

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(a)redhat.com&gt; ... > 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.

And just use cfg->glusterDebugLevel directly here instead of passing it via disk->src->debug_level.

> + } > + > 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.

... > 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.

... Jirka