[libvirt] [PATCH v5 0/2] qemu/gluster: add option for tuning debug logging level

This series run basic sanity and other tests: 1. make syntax-check 2. make check 3. VIR_TEST_VERBOSE=1 ./tests/qemuargv2xmltest 4. VIR_TEST_VERBOSE=1 ./tests/qemuxml2argvtest 5. VIR_TEST_VERBOSE=1 ./tests/qemucapabilitiestest v5: Avoild copying variable glusterDebugLevel to disk->src (virStorageSourcePtri), instead get debug_level from cfg as per Jiri Denemark review on v4 Thanks Jirka 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 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 | 19 +++++++++++++++---- src/qemu/qemu_command.h | 1 + src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_hotplug.c | 6 +++--- .../qemuargv2xml-disk-drive-network-gluster.args | 7 ++++--- .../qemuxml2argv-disk-drive-network-gluster.args | 12 ++++++------ tests/qemuxml2argvtest.c | 3 ++- 12 files changed, 64 insertions(+), 17 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 | 15 ++++++++++++--- src/qemu/qemu_command.h | 1 + src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_hotplug.c | 6 +++--- .../qemuargv2xml-disk-drive-network-gluster.args | 7 ++++--- .../qemuxml2argv-disk-drive-network-gluster.args | 12 ++++++------ 9 files changed, 53 insertions(+), 15 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..3ce609d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1314,6 +1314,7 @@ qemuDiskBusNeedsDeviceArg(int bus) static int qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, + virQEMUDriverConfigPtr cfg, virBufferPtr buf) { int actualType = virStorageSourceGetActualType(disk->src); @@ -1383,6 +1384,11 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, } virBufferAddLit(buf, ","); + if (disk->src && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) { + virBufferAsprintf(buf, "file.debug=%d,", cfg->glusterDebugLevel); + } + 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, @@ -1417,6 +1423,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, char * qemuBuildDriveStr(virDomainDiskDefPtr disk, + virQEMUDriverConfigPtr cfg, bool bootable, virQEMUCapsPtr qemuCaps) { @@ -1508,7 +1515,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, break; } - if (qemuBuildDriveSourceStr(disk, &opt) < 0) + if (qemuBuildDriveSourceStr(disk, cfg, &opt) < 0) goto error; if (emitDeviceSyntax) @@ -2177,6 +2184,7 @@ qemuBuildDriveDevStr(const virDomainDef *def, static int qemuBuildDiskDriveCommandLine(virCommandPtr cmd, + virQEMUDriverConfigPtr cfg, const virDomainDef *def, virQEMUCapsPtr qemuCaps) { @@ -2255,8 +2263,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-drive"); - if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps))) + if (!(optstr = qemuBuildDriveStr(disk, cfg, driveBoot, qemuCaps))) return -1; + virCommandAddArg(cmd, optstr); VIR_FREE(optstr); @@ -9613,7 +9622,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_command.h b/src/qemu/qemu_command.h index 0c0b8f1..79f0444 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -106,6 +106,7 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk); /* Both legacy & current support */ char *qemuBuildDriveStr(virDomainDiskDefPtr disk, + virQEMUDriverConfigPtr cfg, bool bootable, virQEMUCapsPtr qemuCaps); 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/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 72dd93b..9cc8917 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -365,7 +365,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error; - if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) + if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps))) goto error; if (!(drivealias = qemuAliasFromDisk(disk))) @@ -646,7 +646,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; - if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) + if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps))) goto error; if (!(drivealias = qemuAliasFromDisk(disk))) @@ -748,7 +748,7 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; - if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) + if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps))) goto error; if (!(drivealias = qemuAliasFromDisk(disk))) 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

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 | 6 ++++-- tests/qemuxml2argvtest.c | 3 ++- 4 files changed, 12 insertions(+), 3 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 3ce609d..5204e4a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1315,7 +1315,8 @@ qemuDiskBusNeedsDeviceArg(int bus) static int qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, virQEMUDriverConfigPtr cfg, - virBufferPtr buf) + virBufferPtr buf, + virQEMUCapsPtr qemuCaps) { int actualType = virStorageSourceGetActualType(disk->src); qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); @@ -1386,6 +1387,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, if (disk->src && disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) virBufferAsprintf(buf, "file.debug=%d,", cfg->glusterDebugLevel); } @@ -1515,7 +1517,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, break; } - if (qemuBuildDriveSourceStr(disk, cfg, &opt) < 0) + if (qemuBuildDriveSourceStr(disk, cfg, &opt, qemuCaps) < 0) goto error; if (emitDeviceSyntax) 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

Review request. -- Prasanna On Thu, Sep 22, 2016 at 1:04 AM, Prasanna Kumar Kalever <prasanna.kalever@redhat.com> wrote:
This series run basic sanity and other tests: 1. make syntax-check 2. make check 3. VIR_TEST_VERBOSE=1 ./tests/qemuargv2xmltest 4. VIR_TEST_VERBOSE=1 ./tests/qemuxml2argvtest 5. VIR_TEST_VERBOSE=1 ./tests/qemucapabilitiestest
v5: Avoild copying variable glusterDebugLevel to disk->src (virStorageSourcePtri), instead get debug_level from cfg as per Jiri Denemark review on v4 Thanks Jirka
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
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 | 19 +++++++++++++++---- src/qemu/qemu_command.h | 1 + src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_hotplug.c | 6 +++--- .../qemuargv2xml-disk-drive-network-gluster.args | 7 ++++--- .../qemuxml2argv-disk-drive-network-gluster.args | 12 ++++++------ tests/qemuxml2argvtest.c | 3 ++- 12 files changed, 64 insertions(+), 17 deletions(-)
-- 2.7.4

On Thu, Sep 22, 2016 at 01:04:17 +0530, Prasanna Kumar Kalever wrote:
This series run basic sanity and other tests: 1. make syntax-check 2. make check 3. VIR_TEST_VERBOSE=1 ./tests/qemuargv2xmltest 4. VIR_TEST_VERBOSE=1 ./tests/qemuxml2argvtest 5. VIR_TEST_VERBOSE=1 ./tests/qemucapabilitiestest
v5: Avoild copying variable glusterDebugLevel to disk->src (virStorageSourcePtri), instead get debug_level from cfg as per Jiri Denemark review on v4 Thanks Jirka
The patches still should be in reverse order and I still don't think that's impossible to do a proper feature check for the qemu capability. I'll pick them up and try to figure it out once I have time. Peter

On Thu, Sep 29, 2016 at 7:49 PM, Peter Krempa <pkrempa@redhat.com> wrote:
On Thu, Sep 22, 2016 at 01:04:17 +0530, Prasanna Kumar Kalever wrote:
This series run basic sanity and other tests: 1. make syntax-check 2. make check 3. VIR_TEST_VERBOSE=1 ./tests/qemuargv2xmltest 4. VIR_TEST_VERBOSE=1 ./tests/qemuxml2argvtest 5. VIR_TEST_VERBOSE=1 ./tests/qemucapabilitiestest
v5: Avoild copying variable glusterDebugLevel to disk->src (virStorageSourcePtri), instead get debug_level from cfg as per Jiri Denemark review on v4 Thanks Jirka
The patches still should be in reverse order and I still don't think that's impossible to do a proper feature check for the qemu capability.
I'll pick them up and try to figure it out once I have time.
Thanks for you email Peter! Sure, and also please do let me know if you feel convenient to see them as one single patch, I can rework on them. Apart from the order/layout, does these patches addresses all the previous reviews? or I'm I still missing something? -- Prasanna
Peter
participants (3)
-
Peter Krempa
-
Prasanna Kalever
-
Prasanna Kumar Kalever