Re: [libvirt] [PATCH v3 2/2] qemu_capabilities: Introduce gluster specific debug capability
On Tue, Sep 20, 2016 at 3:36 PM, Peter Krempa wrote: > On Tue, Sep 20, 2016 at 15:23:04 +0530, Prasanna Kumar Kalever wrote: >> Teach qemu driver to detect whether qemu supports this configuration >> factor or not. >> >> Signed-off-by: Prasanna Kumar Kalever >> --- >> src/qemu/qemu_capabilities.c | 6 ++ >> src/qemu/qemu_capabilities.h | 3 +++ >> src/qemu/qemu_command.c | 13 - >> tests/qemuxml2argvtest.c | 3 ++- >> 4 files changed, 19 insertions(+), 6 deletions(-) >> >> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c >> index c71b4dc..c4350d5 100644 >> --- a/src/qemu/qemu_capabilities.c >> +++ b/src/qemu/qemu_capabilities.c >> @@ -344,6 +344,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, >>"query-hotpluggable-cpus", >> >>"virtio-net.rx_queue_size", /* 235 */ >> + >> + "debug", /* 236 */ > > This is a VERY bad name for a feature flag. Also please follow the > pattern in the file. It's 5 entries per block, very easy to spot. Peter, I'm pretty bad at naming something, can either of you or Daniel can please suggest one, that will be great. Apologies, 5 block entry just missed my eyes or I might have overlooked at it. > >> ); >> >> >> @@ -2496,6 +2498,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); > > Version checks are bad. Is there anything you can base this on that can > be detected? Version checks break with backports. I don't think we have a better choice w.r.t checks ? > >> + >> return 0; >> } >> >> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h >> index 26ac1fa..ac9f4c8 100644 >> --- a/src/qemu/qemu_capabilities.h >> +++ b/src/qemu/qemu_capabilities.h >> @@ -379,6 +379,9 @@ typedef enum { >> /* 235 */ >> QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */ >> >> +/* 236 */ >> +QEMU_CAPS_GLUSTER_DEBUG_LEVEL, /* -drive debug={0..9} */ > > You are breaking the pattern. Yeah, thank you! > >> + >> 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..ad0f1b2 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) > > Misaligned. > >> { >> 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); > > This patch fails make check. You forgot to bundle in some changes. > > Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/l
Re: [libvirt] [PATCH v3 2/2] qemu_capabilities: Introduce gluster specific debug capability
On Tue, Sep 20, 2016 at 15:23:04 +0530, Prasanna Kumar Kalever wrote: > Teach qemu driver to detect whether qemu supports this configuration > factor or not. > > Signed-off-by: Prasanna Kumar Kalever > --- > src/qemu/qemu_capabilities.c | 6 ++ > src/qemu/qemu_capabilities.h | 3 +++ > src/qemu/qemu_command.c | 13 - > tests/qemuxml2argvtest.c | 3 ++- > 4 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index c71b4dc..c4350d5 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -344,6 +344,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, >"query-hotpluggable-cpus", > >"virtio-net.rx_queue_size", /* 235 */ > + > + "debug", /* 236 */ This is a VERY bad name for a feature flag. Also please follow the pattern in the file. It's 5 entries per block, very easy to spot. > ); > > > @@ -2496,6 +2498,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); Version checks are bad. Is there anything you can base this on that can be detected? Version checks break with backports. > + > return 0; > } > > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index 26ac1fa..ac9f4c8 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -379,6 +379,9 @@ typedef enum { > /* 235 */ > QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */ > > +/* 236 */ > +QEMU_CAPS_GLUSTER_DEBUG_LEVEL, /* -drive debug={0..9} */ You are breaking the pattern. > + > 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..ad0f1b2 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) Misaligned. > { > 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); This patch fails make check. You forgot to bundle in some changes. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 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 --- src/qemu/qemu_capabilities.c | 6 ++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_command.c | 13 - tests/qemuxml2argvtest.c | 3 ++- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c71b4dc..c4350d5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -344,6 +344,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-hotpluggable-cpus", "virtio-net.rx_queue_size", /* 235 */ + + "debug", /* 236 */ ); @@ -2496,6 +2498,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..ac9f4c8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -379,6 +379,9 @@ typedef enum { /* 235 */ QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */ +/* 236 */ +QEMU_CAPS_GLUSTER_DEBUG_LEVEL, /* -drive debug={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..ad0f1b2 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list