[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v6 06/13] qemu: Add qemu command line generation for a VxHS block device




On 08/31/2017 06:39 AM, Peter Krempa wrote:
> On Wed, Aug 30, 2017 at 18:46:06 -0400, John Ferlan wrote:
>> From: Ashish Mittal <Ashish Mittal veritas com>
>>
>> The VxHS block device will only use the newer formatting options and
>> avoid the legacy URI syntax.
>>
>> An excerpt for a sample QEMU command line is:
>>
>>   -drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>>    file.server.0.type=tcp,file.server.0.host=192.168.0.1,\
>>    file.server.0.port=9999,format=raw,if=none,id=drive-virtio-disk0,cache=none \
>>   -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
>>    id=virtio-disk0
>>
>> Update qemuxml2argvtest with a simple test.
>>
>> Signed-off-by: Ashish Mittal <Ashish Mittal veritas com>
>> Signed-off-by: John Ferlan <jferlan redhat com>
>> ---
> 
> [...]
> 
>>  src/qemu/qemu_block.c                              | 48 +++++++++++++++++++++-
>>  src/qemu/qemu_block.h                              |  3 +-
>>  src/qemu/qemu_command.c                            | 12 +++++-
>>  src/qemu/qemu_parse_command.c                      | 16 +++++++-
>>  .../qemuxml2argv-disk-drive-network-vxhs.args      | 27 ++++++++++++
>>  tests/qemuxml2argvtest.c                           |  1 +
>>  6 files changed, 101 insertions(+), 6 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
>>
>> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
>> index d07269f..cb765ab 100644
>> --- a/src/qemu/qemu_block.c
>> +++ b/src/qemu/qemu_block.c
>> @@ -482,6 +482,45 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src)
>>  }
>>  
>>  
>> +static virJSONValuePtr
>> +qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src,
>> +                                   virQEMUCapsPtr qemuCaps)
> 
> Please don't drag 'caps' into the options formatter. Add a function that
> validates whether the disk backend is supported (it should accept a
> virStorageSourcePtr and be called somewhere in
> qemuProcessStartValidate).
> 
> The 'json' syntax may be used even for qemu-img's arguments and thus
> would make this non-reusable.
> 
> Rest looks okay.
> 

So essentially, if the attached patch got merged?  I can update the
series again eventually, but figured I'd get 'buy in' first on the approach.

This will cause merge conflicts in/by patch11 though where I had also
added a diskAlias argument. I suppose I could go with private data
approach too - adding the alias to _qemuDomainDiskPrivate during
qemuDomainPrepareDiskSourceTLS (from patch10)...

Tks -

John
>From 4cf261bd4055ec50866af706ace94756ef6a4d76 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan redhat com>
Date: Thu, 31 Aug 2017 07:55:23 -0400
Subject: [PATCH] Merge these into patch6?

Signed-off-by: John Ferlan <jferlan redhat com>
---
 src/qemu/qemu_block.c   | 15 +++------------
 src/qemu/qemu_block.h   |  3 +--
 src/qemu/qemu_command.c |  2 +-
 src/qemu/qemu_process.c | 26 ++++++++++++++++++++++++++
 4 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index cb765ab..f5269fb 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -483,20 +483,12 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src)
 
 
 static virJSONValuePtr
-qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src,
-                                   virQEMUCapsPtr qemuCaps)
+qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
 {
     const char *protocol = virStorageNetProtocolTypeToString(src->protocol);
     virJSONValuePtr server = NULL;
     virJSONValuePtr ret = NULL;
 
-
-    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VXHS)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("VxHS protocol is not supported with this QEMU binary"));
-        return NULL;
-    }
-
     if (src->nhosts != 1) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("VxHS protocol accepts only one host"));
@@ -529,8 +521,7 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src,
  * storage source. Returns NULL on error and reports an appropriate error message.
  */
 virJSONValuePtr
-qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src,
-                                      virQEMUCapsPtr qemuCaps)
+qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src)
 {
     int actualType = virStorageSourceGetActualType(src);
     virJSONValuePtr fileprops = NULL;
@@ -553,7 +544,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src,
             break;
 
         case VIR_STORAGE_NET_PROTOCOL_VXHS:
-            if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src, qemuCaps)))
+            if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src)))
                 goto cleanup;
             break;
 
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 90f78e3..f0a2c9a 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -54,7 +54,6 @@ virHashTablePtr
 qemuBlockGetNodeData(virJSONValuePtr data);
 
 virJSONValuePtr
-qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src,
-                                      virQEMUCapsPtr qemuCaps);
+qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src);
 
 #endif /* __QEMU_BLOCK_H__ */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3205a59..b9e2ab3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1353,7 +1353,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk,
     int ret = -1;
 
     if (qemuDiskSourceNeedsProps(disk->src) &&
-        !(srcprops = qemuBlockStorageSourceGetBackendProps(disk->src, qemuCaps)))
+        !(srcprops = qemuBlockStorageSourceGetBackendProps(disk->src)))
         goto cleanup;
 
     if (!srcprops &&
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 364c359..e61fe7f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4582,6 +4582,29 @@ qemuProcessStartValidateShmem(virDomainObjPtr vm)
 
 
 static int
+qemuProcessStartValidateDisks(virDomainObjPtr vm,
+                              virQEMUCapsPtr qemuCaps)
+{
+    size_t i;
+
+    for (i = 0; i < vm->def->ndisks; i++) {
+        virStorageSourcePtr src = vm->def->disks[i]->src;
+
+        if (src->type == VIR_STORAGE_TYPE_NETWORK &&
+            src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
+            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VXHS)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("VxHS protocol is not supported with this "
+                             "QEMU binary"));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
+static int
 qemuProcessStartValidateXML(virQEMUDriverPtr driver,
                             virDomainObjPtr vm,
                             virQEMUCapsPtr qemuCaps,
@@ -4663,6 +4686,9 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
     if (qemuProcessStartValidateShmem(vm) < 0)
         return -1;
 
+    if (qemuProcessStartValidateDisks(vm, qemuCaps) < 0)
+        return -1;
+
     VIR_DEBUG("Checking for any possible (non-fatal) issues");
 
     qemuProcessStartWarnShmem(vm);
-- 
2.9.5


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]