On Tue, Sep 20, 2016 at 3:36 PM, Peter Krempa <pkrempa@xxxxxxxxxx> 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 <prasanna.kalever@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list