[libvirt] [PATCH v4 0/2] qemu/gluster: add option for tuning debug logging level
v4: Address comments by peter on v3 Mostly betterments in naming variables and grouping patterns in Caps by 5 Thanks to Peter v3: Add patch 2/2 which address capability checks Changed 'glusterfs_debug_level' to 'gluster_debug_level' agreeing to Peter Made changes in libvirtd_qemu.aug Thanks to Peter & Daniel v2: Modify test cases and syntax check changes as suggested by Peter in v1. Rename qemu_gfapi_debuglevel to glusterfs_debug_level as per Daniel comments. Fix to make debug_level changes effects on URI along with JSON. TODO: * changes in libvirtd_qemu.aug Which I don't understand for now * comment on debug_level variable in storage source Not sure what is the right place * Capablities check v1: Initial post *** BLURB HERE *** Prasanna Kumar Kalever (2): qemu/gluster: add option for tuning debug logging level qemu_capabilities: Introduce gluster specific debug capability src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 20 ++++++++++++++++++++ src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 21 ++++++++++++++++++--- src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/util/virstoragefile.h | 2 ++ .../qemuargv2xml-disk-drive-network-gluster.args | 7 ++++--- .../qemuxml2argv-disk-drive-network-gluster.args | 12 ++++++------ tests/qemuxml2argvtest.c | 3 ++- 11 files changed, 65 insertions(+), 13 deletions(-) -- 2.7.4
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@redhat.com> --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 20 ++++++++++++++++++++ src/qemu/qemu_command.c | 14 +++++++++++++- src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/util/virstoragefile.h | 2 ++ .../qemuargv2xml-disk-drive-network-gluster.args | 7 ++++--- .../qemuxml2argv-disk-drive-network-gluster.args | 12 ++++++------ 8 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 988201e..7cec1c2 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -104,6 +104,8 @@ module Libvirtd_qemu = let nvram_entry = str_array_entry "nvram" + let gluster_debug_level_entry = int_entry "gluster_debug_level" + (* Each entry in the config is one of the following ... *) let entry = default_tls_entry | vnc_entry @@ -119,6 +121,7 @@ module Libvirtd_qemu = | network_entry | log_entry | nvram_entry + | gluster_debug_level_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index e4c2aae..156185a 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -621,3 +621,23 @@ # rollover when a size limit is hit. # #stdio_handler = "logd" + +# Qemu gluster libgfapi log level, debug levels are 0-9, with 9 being the +# most verbose, and 0 representing no debugging output. +# +# 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 +# +# Defaults to 4 +# +# gluster_debug_level = 9 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); + } + 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); @@ -9613,7 +9625,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildDiskDriveCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildDiskDriveCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error; if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dad8334..8ee99ba 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -311,6 +311,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->seccompSandbox = -1; cfg->logTimestamp = true; + cfg->glusterDebugLevel = 4; cfg->stdioLogD = true; #ifdef DEFAULT_LOADER_NVRAM @@ -780,6 +781,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } } + if (virConfGetValueUInt(conf, "gluster_debug_level", &cfg->glusterDebugLevel) < 0) + goto cleanup; ret = 0; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d8232cc..52e32d6 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -190,6 +190,7 @@ struct _virQEMUDriverConfig { virFirmwarePtr *firmwares; size_t nfirmwares; + unsigned int glusterDebugLevel; }; /* Main driver state */ 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 */ diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-gluster.args b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-gluster.args index f560308..deec7a7 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-gluster.args +++ b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-gluster.args @@ -16,9 +16,10 @@ QEMU_AUDIO_DRV=none \ -no-acpi \ -boot c \ -usb \ --drive file=gluster://example.org:6000/Volume1/Image,format=raw,if=virtio \ --drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,format=raw,\ -if=virtio' \ +-drive file=gluster://example.org:6000/Volume1/Image,file.debug=4,format=raw,\ +if=virtio \ +-drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,file.debug=4,\ +format=raw,if=virtio' \ -net none \ -serial none \ -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args index 634ed75..cd5294e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-gluster.args @@ -17,18 +17,18 @@ QEMU_AUDIO_DRV=none \ -no-acpi \ -boot c \ -usb \ --drive file=gluster://example.org:6000/Volume1/Image,format=raw,if=none,\ -id=drive-virtio-disk0 \ +-drive file=gluster://example.org:6000/Volume1/Image,file.debug=4,format=raw,\ +if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\ id=virtio-disk0 \ --drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,format=raw,\ -if=none,id=drive-virtio-disk1' \ +-drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,file.debug=4,\ +format=raw,if=none,id=drive-virtio-disk1' \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\ id=virtio-disk1 \ -drive file.driver=gluster,file.volume=Volume3,file.path=/Image.qcow2,\ file.server.0.type=tcp,file.server.0.host=example.org,file.server.0.port=6000,\ file.server.1.type=tcp,file.server.1.host=example.org,file.server.1.port=24007,\ -file.server.2.type=unix,file.server.2.socket=/path/to/sock,format=qcow2,\ -if=none,id=drive-virtio-disk2 \ +file.server.2.type=unix,file.server.2.socket=/path/to/sock,file.debug=4,\ +format=qcow2,if=none,id=drive-virtio-disk2 \ -device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk2,\ id=virtio-disk2 -- 2.7.4
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@redhat.com> ... 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
On Wed, Sep 21, 2016 at 7:28 PM, Jiri Denemark <jdenemar@redhat.com> 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@redhat.com> ... 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
Teach qemu driver to detect whether qemu supports this configuration factor or not. Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 13 ++++++++----- tests/qemuxml2argvtest.c | 3 ++- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c71b4dc..721e1ce 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -344,6 +344,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-hotpluggable-cpus", "virtio-net.rx_queue_size", /* 235 */ + "gluster.debug_level", ); @@ -2496,6 +2497,10 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, qemuMonitorSupportsActiveCommit(mon)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT); + /* Since qemu 2.7.0 */ + if (qemuCaps->version >= 2007000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL); + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 26ac1fa..ce0fd95 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -378,6 +378,7 @@ typedef enum { /* 235 */ QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */ + QEMU_CAPS_GLUSTER_DEBUG_LEVEL, /* -drive gluster.debug_level={0..9} */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; 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); } 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; } if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps))) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index dbb0e4d..b29a63b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -832,7 +832,8 @@ mymain(void) DO_TEST("disk-drive-network-iscsi-lun", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_BLOCK); - DO_TEST("disk-drive-network-gluster", NONE); + DO_TEST("disk-drive-network-gluster", + QEMU_CAPS_GLUSTER_DEBUG_LEVEL); DO_TEST("disk-drive-network-rbd", NONE); DO_TEST("disk-drive-network-sheepdog", NONE); DO_TEST("disk-drive-network-rbd-auth", NONE); -- 2.7.4
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@redhat.com> ... 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.
}
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
On Wed, Sep 21, 2016 at 7:21 PM, Jiri Denemark <jdenemar@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@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
participants (3)
-
Jiri Denemark -
Prasanna Kalever -
Prasanna Kumar Kalever