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

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 <jferlan(a)redhat.com <mailto:jferlan@redhat.com>> wrote: From: Ashish Mittal <Ashish.Mittal(a)veritas.com <mailto: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(a)veritas.com <mailto:Ashish.Mittal@veritas.com>> Signed-off-by: John Ferlan <jferlan(a)redhat.com <mailto:jferlan@redhat.com>> ---  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:9999}]} +     */ +    if (virJSONValueObjectCreate(&ret, +                                 "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(&buf);              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:")) { +                    virReportError(VIR_ERR_INTERNAL_ERROR, +                                   _("VxHS protocol does not support URI syntax '%s'"), +                                   def->src->path); +                    goto error;                  } else {                      def->src->type = VIR_STORAGE_TYPE_FILE;                  } @@ -1944,6 +1949,10 @@ qemuParseCommandLine(virCapsPtr caps,                  disk->src->type = VIR_STORAGE_TYPE_NETWORK;                  disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_SHEEPDOG;                  val += strlen("sheepdog:"); +            } else if (STRPREFIX(val, "vxhs:")) { +                disk->src->type = VIR_STORAGE_TYPE_NETWORK; +                disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS; +                val += strlen("vxhs:");              } else {                  disk->src->type = VIR_STORAGE_TYPE_FILE;              } @@ -2020,13 +2029,18 @@ qemuParseCommandLine(virCapsPtr caps,                          goto error;                      break; +                case VIR_STORAGE_NET_PROTOCOL_VXHS: +                    virReportError(VIR_ERR_INTERNAL_ERROR, +                                   _("VxHS protocol does not support URI " +                                   "syntax '%s'"), disk->src->path); +                    goto error; +                    break;                  case VIR_STORAGE_NET_PROTOCOL_HTTP:                  case VIR_STORAGE_NET_PROTOCOL_HTTPS:                  case VIR_STORAGE_NET_PROTOCOL_FTP:                  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_LAST:                  case VIR_STORAGE_NET_PROTOCOL_NONE:                      /* ignored for now */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 364c359..0ed5d04 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4582,6 +4582,32 @@ 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; + +        /* This is a best effort check as we can only check if the command +         * option exists, but we cannot determine whether the running QEMU +         * was build with '--enable-vxhs'. */ +        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 +4689,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); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args new file mode 100644 index 0000000..86da1af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args @@ -0,0 +1,27 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-cpu qemu32 \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-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 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 18f06e5..b92ded8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -931,6 +931,7 @@ mymain(void)  # endif      DO_TEST("disk-drive-network-rbd-ipv6", NONE);      DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE); +    DO_TEST("disk-drive-network-vxhs", QEMU_CAPS_VXHS);      DO_TEST("disk-drive-no-boot",              QEMU_CAPS_BOOTINDEX);      DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid", -- 2.9.5