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

2017-09-13 Thread John Ferlan 


On 09/12/2017 01:56 PM, ashish mittal wrote:
> Hi,
> 
> Updating the list with some test results of the base VxHS functionality
> (without TLS), and some changes resulting therefrom -
> 
> (A) Qemu did not work with the
> changed file.server.0.host=127.0.0.1 syntax. Threw an error "Parameter
> 'server.host' is missing". Whereas the old syntax file.server.host=..
> works fine. VxHS protocol accepts only one host, therefore during the
> qemu development, it was decided to drop the redundant index 0.
> 
> (B) In response to (1), we are reverting the syntax back to the
> format file.server.host=1.2.3.4. I have tested with this modification
> and results look good. John has kindly offered to refine those changes
> and include it in the next patch series.
> 
> (C) My testing was focussed on making sure that the libvirt changes work
> with qemu and we are able to read/write to a VxHS disk.
> Essentially I wanted to check libvirt <=> qemu <=> vxhs functionality on
> a real guest VM.
> 
> Here's what I did -
> 
>  1. On my physical system running FC23 stock qemu and libvirt.
>  2. Built and installed the latest VxHS libqnio from github repo.
>  3. Created a test VxHS file /tmp/test_vxhs_disk_1 that qemu can
> write/read to.
>  4. Started the test VxHS server.
>  5. Built and installed the latest qemu after cloning from master repo
> enabling vxhs support.
>  6. Ensured that I was able to start one of my existing VM using my
> custom build qemu.
>  7. Was able to start the guest VM directly using the qemu command line
> after adding the vxhs device. This is what I used to check the
> file.server.host syntax.
>  8. Built and installed latest libvirt with our patches (including the
> change to generate file.server.host)
>  9. Did a virsh edit on my VM and added a VxHS device to the existing XML.
> 10. Started and logged in to the VM. 
> 11. Created a new label on the disk hosted by /tmp/test_vxhs_disk_1 vxhs
> server. 
> 12. Created ext2fs on the disk. Mounted it and copied/removed some files
> from the FS.
> 
> (D) Tested hot plug/unplug using virsh attach-device/detach-device
> back-to-back and this works fine.
> 
> IMO, patches 1-6 with the above adjustments are good to go.
> 
> Regards,
> Ashish
> 

Given the results of testing up through this point... Unless there's
some sort of objection, I'll plan to push the part of this series
upstream some time my tomorrow in the afternoon.

That still leaves the TLS testing and reviewing to be done in order to
push the remainder of the patches. Once pushed, I'll post a v8 that
includes just the TLS work.


John

> 
> On Fri, Sep 1, 2017 at 10:09 AM, John Ferlan  > wrote:
> 
> From: Ashish Mittal  >
> 
> 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=,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  >
> Signed-off-by: John Ferlan  >
> ---
>  src/qemu/qemu_block.c                              | 37
> +-
>  src/qemu/qemu_command.c                            | 10 +-
>  src/qemu/qemu_parse_command.c                      | 16 +-
>  src/qemu/qemu_process.c                            | 29
> +
>  .../qemuxml2argv-disk-drive-network-vxhs.args      | 27
> 
>  tests/qemuxml2argvtest.c                           |  1 +
>  6 files changed, 117 insertions(+), 3 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..f5269fb 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -482,6 +482,37 @@
> qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src)
>  }
> 
> 
> +static virJSONValuePtr
> +qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
> +{
> +    const char *protocol =
> virStorageNetProtocolTypeToString(src->protocol);
> +    virJSONValuePtr server = NULL;
> +    virJSONValuePtr ret = NULL;
> +
> +    if (src->nhosts != 1) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("VxHS protocol accepts only one host"));
> +       

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

2017-09-12 Thread ashish mittal 
Hi,

Updating the list with some test results of the base VxHS functionality
(without TLS), and some changes resulting therefrom -

(A) Qemu did not work with the changed file.server.0.host=127.0.0.1 syntax.
Threw an error "Parameter 'server.host' is missing". Whereas the old syntax
file.server.host=.. works fine. VxHS protocol accepts only one host,
therefore during the qemu development, it was decided to drop the redundant
index 0.

(B) In response to (1), we are reverting the syntax back to the format
file.server.host=1.2.3.4.
I have tested with this modification and results look good. John has kindly
offered to refine those changes and include it in the next patch series.

(C) My testing was focussed on making sure that the libvirt changes work
with qemu and we are able to read/write to a VxHS disk.
Essentially I wanted to check libvirt <=> qemu <=> vxhs functionality on a
real guest VM.

Here's what I did -


   1. On my physical system running FC23 stock qemu and libvirt.
   2. Built and installed the latest VxHS libqnio from github repo.
   3. Created a test VxHS file /tmp/test_vxhs_disk_1 that qemu can
   write/read to.
   4. Started the test VxHS server.
   5. Built and installed the latest qemu after cloning from master repo
   enabling vxhs support.
   6. Ensured that I was able to start one of my existing VM using my
   custom build qemu.
   7. Was able to start the guest VM directly using the qemu command line
   after adding the vxhs device. This is what I used to check the
   file.server.host syntax.
   8. Built and installed latest libvirt with our patches (including the
   change to generate file.server.host)
   9. Did a virsh edit on my VM and added a VxHS device to the existing XML.
   10. Started and logged in to the VM.
   11. Created a new label on the disk hosted by /tmp/test_vxhs_disk_1 vxhs
   server.
   12. Created ext2fs on the disk. Mounted it and copied/removed some files
   from the FS.

(D) Tested hot plug/unplug using virsh attach-device/detach-device
back-to-back and this works fine.

IMO, patches 1-6 with the above adjustments are good to go.

Regards,
Ashish


On Fri, Sep 1, 2017 at 10:09 AM, John Ferlan  wrote:

> From: Ashish Mittal 
>
> 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=,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 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_block.c  | 37
> +-
>  src/qemu/qemu_command.c| 10 +-
>  src/qemu/qemu_parse_command.c  | 16 +-
>  src/qemu/qemu_process.c| 29 +
>  .../qemuxml2argv-disk-drive-network-vxhs.args  | 27 
>  tests/qemuxml2argvtest.c   |  1 +
>  6 files changed, 117 insertions(+), 3 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..f5269fb 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -482,6 +482,37 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr
> src)
>  }
>
>
> +static virJSONValuePtr
> +qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
> +{
> +const char *protocol = virStorageNetProtocolTypeToStr
> ing(src->protocol);
> +virJSONValuePtr server = NULL;
> +virJSONValuePtr ret = NULL;
> +
> +if (src->nhosts != 1) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("VxHS protocol accepts only one host"));
> +return NULL;
> +}
> +
> +if (!(server = qemuBlockStorageSourceBuildHostsJSONSocketAddress(src,
> true)))
> +return NULL;
> +
> +/* VxHS disk specification example:
> + * { driver:"vxhs",
> + *   vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
> + *   server:[{type:"tcp", host:"1.2.3.4", port:}]}
> + */
> +if (virJSONValueObjectCreate(,
> + "s:driver", protocol,
> + "s:vdisk-id", src->path,
> + "a:server", server, NULL) < 0)
> +virJSONValueFree(server);
> +
> +return ret;
> +}
> +
> +
>  /**
>   * qemuBlockStorageSourceGetBackendProps:
>   * @src: disk source
> @@ -512,6 +543,11 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr
> src)
>  goto cleanup;
>

[libvirt] [PATCH v7 06/11] qemu: Add qemu command line generation for a VxHS block device

2017-09-01 Thread John Ferlan 
From: Ashish Mittal 

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=,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 
Signed-off-by: John Ferlan 
---
 src/qemu/qemu_block.c  | 37 +-
 src/qemu/qemu_command.c| 10 +-
 src/qemu/qemu_parse_command.c  | 16 +-
 src/qemu/qemu_process.c| 29 +
 .../qemuxml2argv-disk-drive-network-vxhs.args  | 27 
 tests/qemuxml2argvtest.c   |  1 +
 6 files changed, 117 insertions(+), 3 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..f5269fb 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -482,6 +482,37 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr 
src)
 }
 
 
+static virJSONValuePtr
+qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
+{
+const char *protocol = virStorageNetProtocolTypeToString(src->protocol);
+virJSONValuePtr server = NULL;
+virJSONValuePtr ret = NULL;
+
+if (src->nhosts != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("VxHS protocol accepts only one host"));
+return NULL;
+}
+
+if (!(server = qemuBlockStorageSourceBuildHostsJSONSocketAddress(src, 
true)))
+return NULL;
+
+/* VxHS disk specification example:
+ * { driver:"vxhs",
+ *   vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
+ *   server:[{type:"tcp", host:"1.2.3.4", port:}]}
+ */
+if (virJSONValueObjectCreate(,
+ "s:driver", protocol,
+ "s:vdisk-id", src->path,
+ "a:server", server, NULL) < 0)
+virJSONValueFree(server);
+
+return ret;
+}
+
+
 /**
  * qemuBlockStorageSourceGetBackendProps:
  * @src: disk source
@@ -512,6 +543,11 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr 
src)
 goto cleanup;
 break;
 
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
+if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src)))
+goto cleanup;
+break;
+
 case VIR_STORAGE_NET_PROTOCOL_NBD:
 case VIR_STORAGE_NET_PROTOCOL_RBD:
 case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
@@ -522,7 +558,6 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr 
src)
 case VIR_STORAGE_NET_PROTOCOL_FTPS:
 case VIR_STORAGE_NET_PROTOCOL_TFTP:
 case VIR_STORAGE_NET_PROTOCOL_SSH:
-case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 break;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b7a896e..b9e2ab3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -991,12 +991,16 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
 ret = virBufferContentAndReset();
 break;
 
+case VIR_STORAGE_NET_PROTOCOL_VXHS:
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("VxHS protocol does not support URI syntax"));
+goto cleanup;
+
 case VIR_STORAGE_NET_PROTOCOL_SSH:
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("'ssh' protocol is not yet supported"));
 goto cleanup;
 
-case VIR_STORAGE_NET_PROTOCOL_VXHS:
 case VIR_STORAGE_NET_PROTOCOL_LAST:
 case VIR_STORAGE_NET_PROTOCOL_NONE:
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1326,6 +1330,10 @@ qemuDiskSourceNeedsProps(virStorageSourcePtr src)
 src->nhosts > 1)
 return true;
 
+if (actualType == VIR_STORAGE_TYPE_NETWORK &&
+src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS)
+return true;
+
 return false;
 }
 
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 9190a37..6286c2e 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -736,6 +736,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
 if (VIR_STRDUP(def->src->path, vdi) < 0)
 goto error;
 }
+} else if (STRPREFIX(def->src->path, "vxhs:")) {
+