8:30 a.m.
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
8:30 a.m.
New subject: [libvirt] [PATCH v4 1/2] qemu/gluster: add option for tuning debug logging level
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>
---
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
8:58 a.m.
New subject: [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>
...
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
2:13 p.m.
New subject: [libvirt] [PATCH v4 1/2] qemu/gluster: add option for tuning debug logging level
On Wed, Sep 21, 2016 at 7:28 PM, Jiri Denemark <jdenemar(a)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(a)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
8:30 a.m.
New subject: [libvirt] [PATCH v4 2/2] qemu_capabilities: Introduce gluster specific debug capability
Teach qemu driver to detect whether qemu supports this configuration
factor or not.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever(a)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
8:51 a.m.
New subject: [libvirt] [PATCH v4 2/2] qemu_capabilities: Introduce gluster specific debug capability
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(a)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
2:22 p.m.
New subject: [libvirt] [PATCH v4 2/2] qemu_capabilities: Introduce gluster specific debug capability
On Wed, Sep 21, 2016 at 7:21 PM, Jiri Denemark <jdenemar(a)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(a)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
3016
days inactive
3016
days old
6 comments
3 participants
participants (3)
-
Jiri Denemark -
Prasanna Kalever -
Prasanna Kumar Kalever