All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs"
@ 2017-04-04  3:48 Ashish Mittal
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ashish Mittal @ 2017-04-04  3:48 UTC (permalink / raw)
  To: qemu-devel, pbonzini, kwolf, armbru, berrange, jcody, famz,
	ashish.mittal, stefanha, Ketan.Nilangekar, jferlan,
	Buddhi.Madhav, Suraj.Singh, Nitin.Jerath, peter.maydell,
	venkatesha.mg, Rakesh.Ranjan, eblake
  Cc: Abhijit.Dey, Ashish Mittal

- Veritas HyperScale block driver in QEMU is designed to provide an accelerated
  IO path from KVM virtual machines to Veritas HyperScale storage service.

- A network IO transfer library that translates block IO from HyperScale block
  driver to a network IO format to send it to Veritas HyperScale storage
  service. This library (libvxhs) has been open sourced and is available on
  github here:  https://github.com/VeritasHyperScale/libqnio.git

Ashish Mittal (2):
  block/vxhs.c: Add support for a new block device type called "vxhs"
  block/vxhs.c: Add qemu-iotests for new block device type "vxhs"

 block/Makefile.objs              |   2 +
 block/trace-events               |  17 ++
 block/vxhs.c                     | 575 +++++++++++++++++++++++++++++++++++++++
 configure                        |  39 +++
 qapi/block-core.json             |  23 +-
 tests/qemu-iotests/common        |   6 +
 tests/qemu-iotests/common.config |  13 +
 tests/qemu-iotests/common.filter |   1 +
 tests/qemu-iotests/common.rc     |  19 ++
 9 files changed, 693 insertions(+), 2 deletions(-)
 create mode 100644 block/vxhs.c

-- 
2.5.5

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-04  3:48 [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Ashish Mittal
@ 2017-04-04  3:48 ` Ashish Mittal
  2017-04-04 15:55   ` Daniel P. Berrange
                     ` (2 more replies)
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs" Ashish Mittal
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 12+ messages in thread
From: Ashish Mittal @ 2017-04-04  3:48 UTC (permalink / raw)
  To: qemu-devel, pbonzini, kwolf, armbru, berrange, jcody, famz,
	ashish.mittal, stefanha, Ketan.Nilangekar, jferlan,
	Buddhi.Madhav, Suraj.Singh, Nitin.Jerath, peter.maydell,
	venkatesha.mg, Rakesh.Ranjan, eblake
  Cc: Abhijit.Dey, Ashish Mittal

Source code for the qnio library that this code loads can be downloaded from:
https://github.com/VeritasHyperScale/libqnio.git

Sample command line using JSON syntax:
./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
-k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
-msg timestamp=on
'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
"server":{"host":"172.172.17.4","port":"9999"}}'

Sample command line using URI syntax:
qemu-img convert -f raw -O raw -n
/var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0

Sample command line using TLS credentials (run in secure mode):
./qemu-io --object
tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
-v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
"vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'

Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
---
v11 changelog:
(1) Replaced InetSocketAddress with InetSocketAddressBase.
(2) Removed access to qemu_uuid.
(3) Removed unnecessary g_strdup()/g_free().
(4) Removed unused acb->qiov.
(5) Changed vxhs_init_and_ref() and vxhs_unref() per suggestion.
(6) Removed unnecessary initializations from local variables.

v10 changelog:
(1) Implemented accepting TLS creds per block device via the CLI
    (see 3rd e.g in commit log). Corresponding changes made to the
    libqnio library.
(2) iio_open() changed to accept TLS creds and use these internally
    to set up SSL connections.
(3) Got rid of hard-coded VXHS_UUID_DEF. qemu_uuid is no longer used
    for authentication in any way.
(4) Removed unnecessary qdict_del(backing_options, str).
(5) Added '*tls-creds' to BlockdevOptionsVxHS.

v9 changelog:
(1) Fixes for all the review comments from v8. I have left the definition
    of VXHS_UUID_DEF unchanged pending a better suggestion.
(2) qcow2 tests now pass on the vxhs test server.
(3) Packaging changes for libvxhs will be checked in to the git repo soon.
(4) I have not moved extern QemuUUID qemu_uuid to a separate header file.

v8 changelog:
(1) Security implementation for libqnio present in branch 'securify'.
    Please use 'securify' branch for building libqnio and testing
    with this patch.
(2) Renamed libqnio to libvxhs.
(3) Pass instance ID to libvxhs for SSL authentication.

v7 changelog:
(1) IO failover code has moved out to the libqnio library.
(2) Fixes for issues reported by Stefan on v6.
(3) Incorporated the QEMUBH patch provided by Stefan.
    This is a replacement for the pipe mechanism used earlier.
(4) Fixes to the buffer overflows reported in libqnio.
(5) Input validations in vxhs.c to prevent any buffer overflows for 
    arguments passed to libqnio.

v6 changelog:
(1) Added qemu-iotests for VxHS as a new patch in the series.
(2) Replaced release version from 2.8 to 2.9 in block-core.json.

v5 changelog:
(1) Incorporated v4 review comments.

v4 changelog:
(1) Incorporated v3 review comments on QAPI changes.
(2) Added refcounting for device open/close.
    Free library resources on last device close.

v3 changelog:
(1) Added QAPI schema for the VxHS driver.

v2 changelog:
(1) Changes done in response to v1 comments.

 block/Makefile.objs  |   2 +
 block/trace-events   |  17 ++
 block/vxhs.c         | 575 +++++++++++++++++++++++++++++++++++++++++++++++++++
 configure            |  39 ++++
 qapi/block-core.json |  23 ++-
 5 files changed, 654 insertions(+), 2 deletions(-)
 create mode 100644 block/vxhs.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index de96f8e..ea95530 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -19,6 +19,7 @@ block-obj-$(CONFIG_LIBNFS) += nfs.o
 block-obj-$(CONFIG_CURL) += curl.o
 block-obj-$(CONFIG_RBD) += rbd.o
 block-obj-$(CONFIG_GLUSTERFS) += gluster.o
+block-obj-$(CONFIG_VXHS) += vxhs.o
 block-obj-$(CONFIG_LIBSSH2) += ssh.o
 block-obj-y += accounting.o dirty-bitmap.o
 block-obj-y += write-threshold.o
@@ -38,6 +39,7 @@ rbd.o-cflags       := $(RBD_CFLAGS)
 rbd.o-libs         := $(RBD_LIBS)
 gluster.o-cflags   := $(GLUSTERFS_CFLAGS)
 gluster.o-libs     := $(GLUSTERFS_LIBS)
+vxhs.o-libs        := $(VXHS_LIBS)
 ssh.o-cflags       := $(LIBSSH2_CFLAGS)
 ssh.o-libs         := $(LIBSSH2_LIBS)
 block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
diff --git a/block/trace-events b/block/trace-events
index 0bc5c0a..7758ec3 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -110,3 +110,20 @@ qed_aio_write_data(void *s, void *acb, int ret, uint64_t offset, size_t len) "s
 qed_aio_write_prefill(void *s, void *acb, uint64_t start, size_t len, uint64_t offset) "s %p acb %p start %"PRIu64" len %zu offset %"PRIu64
 qed_aio_write_postfill(void *s, void *acb, uint64_t start, size_t len, uint64_t offset) "s %p acb %p start %"PRIu64" len %zu offset %"PRIu64
 qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s %p acb %p ret %d offset %"PRIu64" len %zu"
+
+# block/vxhs.c
+vxhs_iio_callback(int error) "ctx is NULL: error %d"
+vxhs_iio_callback_chnfail(int err, int error) "QNIO channel failed, no i/o %d, %d"
+vxhs_iio_callback_unknwn(int opcode, int err) "unexpected opcode %d, errno %d"
+vxhs_aio_rw_invalid(int req) "Invalid I/O request iodir %d"
+vxhs_aio_rw_ioerr(char *guid, int iodir, uint64_t size, uint64_t off, void *acb, int ret, int err) "IO ERROR (vDisk %s) FOR : Read/Write = %d size = %lu offset = %lu ACB = %p. Error = %d, errno = %d"
+vxhs_get_vdisk_stat_err(char *guid, int ret, int err) "vDisk (%s) stat ioctl failed, ret = %d, errno = %d"
+vxhs_get_vdisk_stat(char *vdisk_guid, uint64_t vdisk_size) "vDisk %s stat ioctl returned size %lu"
+vxhs_complete_aio(void *acb, uint64_t ret) "aio failed acb %p ret %ld"
+vxhs_parse_uri_filename(const char *filename) "URI passed via bdrv_parse_filename %s"
+vxhs_open_vdiskid(const char *vdisk_id) "Opening vdisk-id %s"
+vxhs_open_hostinfo(char *of_vsa_addr, int port) "Adding host %s:%d to BDRVVXHSState"
+vxhs_open_iio_open(const char *host) "Failed to connect to storage agent on host %s"
+vxhs_parse_uri_hostinfo(char *host, int port) "Host: IP %s, Port %d"
+vxhs_close(char *vdisk_guid) "Closing vdisk %s"
+vxhs_get_creds(const char *cacert, const char *client_key, const char *client_cert) "cacert %s, client_key %s, client_cert %s"
diff --git a/block/vxhs.c b/block/vxhs.c
new file mode 100644
index 0000000..9ffe9d3
--- /dev/null
+++ b/block/vxhs.c
@@ -0,0 +1,575 @@
+/*
+ * QEMU Block driver for Veritas HyperScale (VxHS)
+ *
+ * Copyright (c) 2017 Veritas Technologies LLC.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include <qnio/qnio_api.h>
+#include <sys/param.h>
+#include "block/block_int.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
+#include "trace.h"
+#include "qemu/uri.h"
+#include "qapi/error.h"
+#include "qemu/uuid.h"
+#include "crypto/tlscredsx509.h"
+
+#define VXHS_OPT_FILENAME           "filename"
+#define VXHS_OPT_VDISK_ID           "vdisk-id"
+#define VXHS_OPT_SERVER             "server"
+#define VXHS_OPT_HOST               "host"
+#define VXHS_OPT_PORT               "port"
+
+/* Only accessed under QEMU global mutex */
+static uint32_t vxhs_ref;
+
+typedef enum {
+    VDISK_AIO_READ,
+    VDISK_AIO_WRITE,
+} VDISKAIOCmd;
+
+/*
+ * HyperScale AIO callbacks structure
+ */
+typedef struct VXHSAIOCB {
+    BlockAIOCB common;
+    int err;
+} VXHSAIOCB;
+
+typedef struct VXHSvDiskHostsInfo {
+    void *dev_handle; /* Device handle */
+    char *host; /* Host name or IP */
+    int port; /* Host's port number */
+} VXHSvDiskHostsInfo;
+
+/*
+ * Structure per vDisk maintained for state
+ */
+typedef struct BDRVVXHSState {
+    VXHSvDiskHostsInfo vdisk_hostinfo; /* Per host info */
+    char *vdisk_guid;
+    char *tlscredsid; /* tlscredsid */
+} BDRVVXHSState;
+
+static void vxhs_complete_aio_bh(void *opaque)
+{
+    VXHSAIOCB *acb = opaque;
+    BlockCompletionFunc *cb = acb->common.cb;
+    void *cb_opaque = acb->common.opaque;
+    int ret = 0;
+
+    if (acb->err != 0) {
+        trace_vxhs_complete_aio(acb, acb->err);
+        ret = (-EIO);
+    }
+
+    qemu_aio_unref(acb);
+    cb(cb_opaque, ret);
+}
+
+/*
+ * Called from a libqnio thread
+ */
+static void vxhs_iio_callback(void *ctx, uint32_t opcode, uint32_t error)
+{
+    VXHSAIOCB *acb = NULL;
+
+    switch (opcode) {
+    case IRP_READ_REQUEST:
+    case IRP_WRITE_REQUEST:
+
+        /*
+         * ctx is VXHSAIOCB*
+         * ctx is NULL if error is QNIOERROR_CHANNEL_HUP
+         */
+        if (ctx) {
+            acb = ctx;
+        } else {
+            trace_vxhs_iio_callback(error);
+            goto out;
+        }
+
+        if (error) {
+            if (!acb->err) {
+                acb->err = error;
+            }
+            trace_vxhs_iio_callback(error);
+        }
+
+        aio_bh_schedule_oneshot(bdrv_get_aio_context(acb->common.bs),
+                                vxhs_complete_aio_bh, acb);
+        break;
+
+    default:
+        if (error == QNIOERROR_HUP) {
+            /*
+             * Channel failed, spontaneous notification,
+             * not in response to I/O
+             */
+            trace_vxhs_iio_callback_chnfail(error, errno);
+        } else {
+            trace_vxhs_iio_callback_unknwn(opcode, error);
+        }
+        break;
+    }
+out:
+    return;
+}
+
+static QemuOptsList runtime_opts = {
+    .name = "vxhs",
+    .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+    .desc = {
+        {
+            .name = VXHS_OPT_FILENAME,
+            .type = QEMU_OPT_STRING,
+            .help = "URI to the Veritas HyperScale image",
+        },
+        {
+            .name = VXHS_OPT_VDISK_ID,
+            .type = QEMU_OPT_STRING,
+            .help = "UUID of the VxHS vdisk",
+        },
+        {
+            .name = "tls-creds",
+            .type = QEMU_OPT_STRING,
+            .help = "ID of the TLS/SSL credentials to use",
+        },
+        { /* end of list */ }
+    },
+};
+
+static QemuOptsList runtime_tcp_opts = {
+    .name = "vxhs_tcp",
+    .head = QTAILQ_HEAD_INITIALIZER(runtime_tcp_opts.head),
+    .desc = {
+        {
+            .name = VXHS_OPT_HOST,
+            .type = QEMU_OPT_STRING,
+            .help = "host address (ipv4 addresses)",
+        },
+        {
+            .name = VXHS_OPT_PORT,
+            .type = QEMU_OPT_NUMBER,
+            .help = "port number on which VxHSD is listening (default 9999)",
+            .def_value_str = "9999"
+        },
+        { /* end of list */ }
+    },
+};
+
+/*
+ * Parse incoming URI and populate *options with the host
+ * and device information
+ */
+static int vxhs_parse_uri(const char *filename, QDict *options)
+{
+    URI *uri = NULL;
+    char *port;
+    int ret = 0;
+
+    trace_vxhs_parse_uri_filename(filename);
+    uri = uri_parse(filename);
+    if (!uri || !uri->server || !uri->path) {
+        uri_free(uri);
+        return -EINVAL;
+    }
+
+    qdict_put(options, VXHS_OPT_SERVER".host", qstring_from_str(uri->server));
+
+    if (uri->port) {
+        port = g_strdup_printf("%d", uri->port);
+        qdict_put(options, VXHS_OPT_SERVER".port", qstring_from_str(port));
+        g_free(port);
+    }
+
+    qdict_put(options, "vdisk-id", qstring_from_str(uri->path));
+
+    trace_vxhs_parse_uri_hostinfo(uri->server, uri->port);
+    uri_free(uri);
+
+    return ret;
+}
+
+static void vxhs_parse_filename(const char *filename, QDict *options,
+                                Error **errp)
+{
+    if (qdict_haskey(options, "vdisk-id") || qdict_haskey(options, "server")) {
+        error_setg(errp, "vdisk-id/server and a file name may not be specified "
+                         "at the same time");
+        return;
+    }
+
+    if (strstr(filename, "://")) {
+        int ret = vxhs_parse_uri(filename, options);
+        if (ret < 0) {
+            error_setg(errp, "Invalid URI. URI should be of the form "
+                       "  vxhs://<host_ip>:<port>/<vdisk-id>");
+        }
+    }
+}
+
+static int vxhs_init_and_ref(void)
+{
+    if (vxhs_ref++ == 0) {
+        if (iio_init(QNIO_VERSION, vxhs_iio_callback)) {
+            return -ENODEV;
+        }
+    }
+    return 0;
+}
+
+static void vxhs_unref(void)
+{
+    if (--vxhs_ref == 0) {
+        iio_fini();
+    }
+}
+
+static void vxhs_get_tls_creds(const char *id, char **cacert,
+                               char **key, char **cert, Error **errp)
+{
+    Object *obj;
+    QCryptoTLSCreds *creds;
+    QCryptoTLSCredsX509 *creds_x509;
+
+    obj = object_resolve_path_component(
+        object_get_objects_root(), id);
+
+    if (!obj) {
+        error_setg(errp, "No TLS credentials with id '%s'",
+                   id);
+        return;
+    }
+
+    creds_x509 = (QCryptoTLSCredsX509 *)
+        object_dynamic_cast(obj, TYPE_QCRYPTO_TLS_CREDS_X509);
+
+    if (!creds_x509) {
+        error_setg(errp, "Object with id '%s' is not TLS credentials",
+                   id);
+        return;
+    }
+
+    creds = &creds_x509->parent_obj;
+
+    if (creds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) {
+        error_setg(errp,
+                   "Expecting TLS credentials with a client endpoint");
+        return;
+    }
+
+    /*
+     * Get the cacert, client_cert and client_key file names.
+     */
+    if (!creds->dir) {
+        error_setg(errp, "TLS object missing 'dir' property value");
+        return;
+    }
+
+    *cacert = g_strdup_printf("%s/%s", creds->dir,
+                              QCRYPTO_TLS_CREDS_X509_CA_CERT);
+    *cert = g_strdup_printf("%s/%s", creds->dir,
+                            QCRYPTO_TLS_CREDS_X509_CLIENT_CERT);
+    *key = g_strdup_printf("%s/%s", creds->dir,
+                           QCRYPTO_TLS_CREDS_X509_CLIENT_KEY);
+}
+
+static int vxhs_open(BlockDriverState *bs, QDict *options,
+                     int bdrv_flags, Error **errp)
+{
+    BDRVVXHSState *s = bs->opaque;
+    void *dev_handlep;
+    QDict *backing_options = NULL;
+    QemuOpts *opts = NULL;
+    QemuOpts *tcp_opts = NULL;
+    char *of_vsa_addr = NULL;
+    Error *local_err = NULL;
+    const char *vdisk_id_opt;
+    const char *server_host_opt;
+    int ret = 0;
+    char *cacert = NULL;
+    char *client_key = NULL;
+    char *client_cert = NULL;
+
+    ret = vxhs_init_and_ref();
+    if (ret < 0) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* Create opts info from runtime_opts and runtime_tcp_opts list */
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
+    tcp_opts = qemu_opts_create(&runtime_tcp_opts, NULL, 0, &error_abort);
+
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* vdisk-id is the disk UUID */
+    vdisk_id_opt = qemu_opt_get(opts, VXHS_OPT_VDISK_ID);
+    if (!vdisk_id_opt) {
+        error_setg(&local_err, QERR_MISSING_PARAMETER, VXHS_OPT_VDISK_ID);
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* vdisk-id may contain a leading '/' */
+    if (strlen(vdisk_id_opt) > UUID_FMT_LEN + 1) {
+        error_setg(&local_err, "vdisk-id cannot be more than %d characters",
+                   UUID_FMT_LEN);
+        ret = -EINVAL;
+        goto out;
+    }
+
+    s->vdisk_guid = g_strdup(vdisk_id_opt);
+    trace_vxhs_open_vdiskid(vdisk_id_opt);
+
+    /* get the 'server.' arguments */
+    qdict_extract_subqdict(options, &backing_options, VXHS_OPT_SERVER".");
+
+    qemu_opts_absorb_qdict(tcp_opts, backing_options, &local_err);
+    if (local_err != NULL) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    server_host_opt = qemu_opt_get(tcp_opts, VXHS_OPT_HOST);
+    if (!server_host_opt) {
+        error_setg(&local_err, QERR_MISSING_PARAMETER,
+                   VXHS_OPT_SERVER"."VXHS_OPT_HOST);
+        ret = -EINVAL;
+        goto out;
+    }
+
+    if (strlen(server_host_opt) > MAXHOSTNAMELEN) {
+        error_setg(&local_err, "server.host cannot be more than %d characters",
+                   MAXHOSTNAMELEN);
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* check if we got tls-creds via the --object argument */
+    s->tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds"));
+    if (s->tlscredsid) {
+        vxhs_get_tls_creds(s->tlscredsid, &cacert, &client_key,
+                           &client_cert, &local_err);
+        if (local_err != NULL) {
+            ret = -EINVAL;
+            goto out;
+        }
+        trace_vxhs_get_creds(cacert, client_key, client_cert);
+    }
+
+    s->vdisk_hostinfo.host = g_strdup(server_host_opt);
+    s->vdisk_hostinfo.port = g_ascii_strtoll(qemu_opt_get(tcp_opts,
+                                                          VXHS_OPT_PORT),
+                                                          NULL, 0);
+
+    trace_vxhs_open_hostinfo(s->vdisk_hostinfo.host,
+                             s->vdisk_hostinfo.port);
+
+    of_vsa_addr = g_strdup_printf("of://%s:%d",
+                                  s->vdisk_hostinfo.host,
+                                  s->vdisk_hostinfo.port);
+
+    /*
+     * Open qnio channel to storage agent if not opened before
+     */
+    dev_handlep = iio_open(of_vsa_addr, s->vdisk_guid, 0,
+                           cacert, client_key, client_cert);
+    if (dev_handlep == NULL) {
+        trace_vxhs_open_iio_open(of_vsa_addr);
+        ret = -ENODEV;
+        goto out;
+    }
+    s->vdisk_hostinfo.dev_handle = dev_handlep;
+
+out:
+    g_free(of_vsa_addr);
+    QDECREF(backing_options);
+    qemu_opts_del(tcp_opts);
+    qemu_opts_del(opts);
+    g_free(cacert);
+    g_free(client_key);
+    g_free(client_cert);
+
+    if (ret < 0) {
+        vxhs_unref();
+        error_propagate(errp, local_err);
+        g_free(s->vdisk_hostinfo.host);
+        g_free(s->vdisk_guid);
+        g_free(s->tlscredsid);
+        s->vdisk_guid = NULL;
+    }
+
+    return ret;
+}
+
+static const AIOCBInfo vxhs_aiocb_info = {
+    .aiocb_size = sizeof(VXHSAIOCB)
+};
+
+/*
+ * This allocates QEMU-VXHS callback for each IO
+ * and is passed to QNIO. When QNIO completes the work,
+ * it will be passed back through the callback.
+ */
+static BlockAIOCB *vxhs_aio_rw(BlockDriverState *bs, int64_t sector_num,
+                               QEMUIOVector *qiov, int nb_sectors,
+                               BlockCompletionFunc *cb, void *opaque,
+                               VDISKAIOCmd iodir)
+{
+    VXHSAIOCB *acb = NULL;
+    BDRVVXHSState *s = bs->opaque;
+    size_t size;
+    uint64_t offset;
+    int iio_flags = 0;
+    int ret = 0;
+    void *dev_handle = s->vdisk_hostinfo.dev_handle;
+
+    offset = sector_num * BDRV_SECTOR_SIZE;
+    size = nb_sectors * BDRV_SECTOR_SIZE;
+    acb = qemu_aio_get(&vxhs_aiocb_info, bs, cb, opaque);
+
+    /*
+     * Initialize VXHSAIOCB.
+     */
+    acb->err = 0;
+
+    iio_flags = IIO_FLAG_ASYNC;
+
+    switch (iodir) {
+    case VDISK_AIO_WRITE:
+            ret = iio_writev(dev_handle, acb, qiov->iov, qiov->niov,
+                             offset, (uint64_t)size, iio_flags);
+            break;
+    case VDISK_AIO_READ:
+            ret = iio_readv(dev_handle, acb, qiov->iov, qiov->niov,
+                            offset, (uint64_t)size, iio_flags);
+            break;
+    default:
+            trace_vxhs_aio_rw_invalid(iodir);
+            goto errout;
+    }
+
+    if (ret != 0) {
+        trace_vxhs_aio_rw_ioerr(s->vdisk_guid, iodir, size, offset,
+                                acb, ret, errno);
+        goto errout;
+    }
+    return &acb->common;
+
+errout:
+    qemu_aio_unref(acb);
+    return NULL;
+}
+
+static BlockAIOCB *vxhs_aio_readv(BlockDriverState *bs,
+                                   int64_t sector_num, QEMUIOVector *qiov,
+                                   int nb_sectors,
+                                   BlockCompletionFunc *cb, void *opaque)
+{
+    return vxhs_aio_rw(bs, sector_num, qiov, nb_sectors, cb,
+                       opaque, VDISK_AIO_READ);
+}
+
+static BlockAIOCB *vxhs_aio_writev(BlockDriverState *bs,
+                                   int64_t sector_num, QEMUIOVector *qiov,
+                                   int nb_sectors,
+                                   BlockCompletionFunc *cb, void *opaque)
+{
+    return vxhs_aio_rw(bs, sector_num, qiov, nb_sectors,
+                       cb, opaque, VDISK_AIO_WRITE);
+}
+
+static void vxhs_close(BlockDriverState *bs)
+{
+    BDRVVXHSState *s = bs->opaque;
+
+    trace_vxhs_close(s->vdisk_guid);
+
+    g_free(s->vdisk_guid);
+    s->vdisk_guid = NULL;
+
+    /*
+     * Close vDisk device
+     */
+    if (s->vdisk_hostinfo.dev_handle) {
+        iio_close(s->vdisk_hostinfo.dev_handle);
+        s->vdisk_hostinfo.dev_handle = NULL;
+    }
+
+    vxhs_unref();
+
+    /*
+     * Free the dynamically allocated host string etc
+     */
+    g_free(s->vdisk_hostinfo.host);
+    g_free(s->tlscredsid);
+    s->tlscredsid = NULL;
+    s->vdisk_hostinfo.host = NULL;
+    s->vdisk_hostinfo.port = 0;
+}
+
+static int64_t vxhs_get_vdisk_stat(BDRVVXHSState *s)
+{
+    int64_t vdisk_size = -1;
+    int ret = 0;
+    void *dev_handle = s->vdisk_hostinfo.dev_handle;
+
+    ret = iio_ioctl(dev_handle, IOR_VDISK_STAT, &vdisk_size, 0);
+    if (ret < 0) {
+        trace_vxhs_get_vdisk_stat_err(s->vdisk_guid, ret, errno);
+        return -EIO;
+    }
+
+    trace_vxhs_get_vdisk_stat(s->vdisk_guid, vdisk_size);
+    return vdisk_size;
+}
+
+/*
+ * Returns the size of vDisk in bytes. This is required
+ * by QEMU block upper block layer so that it is visible
+ * to guest.
+ */
+static int64_t vxhs_getlength(BlockDriverState *bs)
+{
+    BDRVVXHSState *s = bs->opaque;
+    int64_t vdisk_size;
+
+    vdisk_size = vxhs_get_vdisk_stat(s);
+    if (vdisk_size < 0) {
+        return -EIO;
+    }
+
+    return vdisk_size;
+}
+
+static BlockDriver bdrv_vxhs = {
+    .format_name                  = "vxhs",
+    .protocol_name                = "vxhs",
+    .instance_size                = sizeof(BDRVVXHSState),
+    .bdrv_file_open               = vxhs_open,
+    .bdrv_parse_filename          = vxhs_parse_filename,
+    .bdrv_close                   = vxhs_close,
+    .bdrv_getlength               = vxhs_getlength,
+    .bdrv_aio_readv               = vxhs_aio_readv,
+    .bdrv_aio_writev              = vxhs_aio_writev,
+};
+
+static void bdrv_vxhs_init(void)
+{
+    bdrv_register(&bdrv_vxhs);
+}
+
+block_init(bdrv_vxhs_init);
diff --git a/configure b/configure
index 4b3b5cd..94f0818 100755
--- a/configure
+++ b/configure
@@ -320,6 +320,7 @@ numa=""
 tcmalloc="no"
 jemalloc="no"
 replication="yes"
+vxhs=""
 
 supported_cpu="no"
 supported_os="no"
@@ -1183,6 +1184,10 @@ for opt do
   ;;
   --enable-replication) replication="yes"
   ;;
+  --disable-vxhs) vxhs="no"
+  ;;
+  --enable-vxhs) vxhs="yes"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -1427,6 +1432,7 @@ disabled with --disable-FEATURE, default is enabled if available:
   xfsctl          xfsctl support
   qom-cast-debug  cast debugging support
   tools           build qemu-io, qemu-nbd and qemu-image tools
+  vxhs            Veritas HyperScale vDisk backend support
 
 NOTE: The object files are built at the place where configure is launched
 EOF
@@ -4777,6 +4783,33 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
+# Veritas HyperScale block driver VxHS
+# Check if libvxhs is installed
+
+if test "$vxhs" != "no" ; then
+  cat > $TMPC <<EOF
+#include <stdint.h>
+#include <qnio/qnio_api.h>
+
+void *vxhs_callback;
+
+int main(void) {
+    iio_init(QNIO_VERSION, vxhs_callback);
+    return 0;
+}
+EOF
+  vxhs_libs="-lvxhs -lssl"
+  if compile_prog "" "$vxhs_libs" ; then
+    vxhs=yes
+  else
+    if test "$vxhs" = "yes" ; then
+      feature_not_found "vxhs block device" "Install libvxhs See github"
+    fi
+    vxhs=no
+  fi
+fi
+
+##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
 
@@ -5142,6 +5175,7 @@ echo "tcmalloc support  $tcmalloc"
 echo "jemalloc support  $jemalloc"
 echo "avx2 optimization $avx2_opt"
 echo "replication support $replication"
+echo "VxHS block device $vxhs"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -5781,6 +5815,11 @@ if test "$pthread_setname_np" = "yes" ; then
   echo "CONFIG_PTHREAD_SETNAME_NP=y" >> $config_host_mak
 fi
 
+if test "$vxhs" = "yes" ; then
+  echo "CONFIG_VXHS=y" >> $config_host_mak
+  echo "VXHS_LIBS=$vxhs_libs" >> $config_host_mak
+fi
+
 if test "$tcg_interpreter" = "yes"; then
   QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 033457c..87fb747 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2108,6 +2108,8 @@
 #
 # Drivers that are supported in block device operations.
 #
+# @vxhs: Since 2.10
+#
 # Since: 2.9
 ##
 { 'enum': 'BlockdevDriver',
@@ -2116,7 +2118,7 @@
             'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
             'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
             'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh',
-            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
+            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] }
 
 ##
 # @BlockdevOptionsFile:
@@ -2866,6 +2868,22 @@
   'data': { '*offset': 'int', '*size': 'int' } }
 
 ##
+# @BlockdevOptionsVxHS:
+#
+# Driver specific block device options for VxHS
+#
+# @vdisk-id:    UUID of VxHS volume
+# @server:      vxhs server IP, port
+# @tls-creds:   TLS credentials ID
+#
+# Since: 2.10
+##
+{ 'struct': 'BlockdevOptionsVxHS',
+  'data': { 'vdisk-id': 'str',
+            'server': 'InetSocketAddressBase',
+            '*tls-creds': 'str' } }
+
+##
 # @BlockdevOptions:
 #
 # Options for creating a block device.  Many options are available for all
@@ -2927,7 +2945,8 @@
       'vhdx':       'BlockdevOptionsGenericFormat',
       'vmdk':       'BlockdevOptionsGenericCOWFormat',
       'vpc':        'BlockdevOptionsGenericFormat',
-      'vvfat':      'BlockdevOptionsVVFAT'
+      'vvfat':      'BlockdevOptionsVVFAT',
+      'vxhs':       'BlockdevOptionsVxHS'
   } }
 
 ##
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v11 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs"
  2017-04-04  3:48 [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Ashish Mittal
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
@ 2017-04-04  3:48 ` Ashish Mittal
  2017-04-04 18:02 ` [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Jeff Cody
  2017-04-20 15:03 ` Jeff Cody
  3 siblings, 0 replies; 12+ messages in thread
From: Ashish Mittal @ 2017-04-04  3:48 UTC (permalink / raw)
  To: qemu-devel, pbonzini, kwolf, armbru, berrange, jcody, famz,
	ashish.mittal, stefanha, Ketan.Nilangekar, jferlan,
	Buddhi.Madhav, Suraj.Singh, Nitin.Jerath, peter.maydell,
	venkatesha.mg, Rakesh.Ranjan, eblake
  Cc: Abhijit.Dey, Ashish Mittal

These changes use a vxhs test server that is a part of the following
repository:
https://github.com/VeritasHyperScale/libqnio.git

Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v11 changelog:
(1) No changes.

v10 changelog:
(1) Redirect o/p of "$QEMU_VXHS -d  $TEST_DIR" to /dev/null

v9 changelog:
(1) Dropped second argument to set_prog_path(). We will pick up the test
    server location from the user's PATH env setting.

v8/v7 changelog:
(1) No changes.

v6 changelog:
(1) Added iotests for VxHS block device.

 tests/qemu-iotests/common        |  6 ++++++
 tests/qemu-iotests/common.config | 13 +++++++++++++
 tests/qemu-iotests/common.filter |  1 +
 tests/qemu-iotests/common.rc     | 19 +++++++++++++++++++
 4 files changed, 39 insertions(+)

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 4d5650d..9c6f972 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -157,6 +157,7 @@ check options
     -ssh                test ssh
     -nfs                test nfs
     -luks               test luks
+    -vxhs               test vxhs
     -xdiff              graphical mode diff
     -nocache            use O_DIRECT on backing file
     -misalign           misalign memory allocations
@@ -260,6 +261,11 @@ testlist options
             xpand=false
             ;;
 
+        -vxhs)
+            IMGPROTO=vxhs
+            xpand=false
+            ;;
+
         -ssh)
             IMGPROTO=ssh
             xpand=false
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 55527aa..c4b51b3 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -105,6 +105,10 @@ if [ -z "$QEMU_NBD_PROG" ]; then
     export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
 fi
 
+if [ -z "$QEMU_VXHS_PROG" ]; then
+    export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
+fi
+
 _qemu_wrapper()
 {
     (
@@ -156,10 +160,19 @@ _qemu_nbd_wrapper()
     )
 }
 
+_qemu_vxhs_wrapper()
+{
+    (
+        echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid"
+        exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
+    )
+}
+
 export QEMU=_qemu_wrapper
 export QEMU_IMG=_qemu_img_wrapper
 export QEMU_IO=_qemu_io_wrapper
 export QEMU_NBD=_qemu_nbd_wrapper
+export QEMU_VXHS=_qemu_vxhs_wrapper
 
 QEMU_IMG_EXTRA_ARGS=
 if [ "$IMGOPTSSYNTAX" = "true" ]; then
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 1040013..c9a2d5c 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -122,6 +122,7 @@ _filter_img_info()
         -e "s#$TEST_DIR#TEST_DIR#g" \
         -e "s#$IMGFMT#IMGFMT#g" \
         -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \
+        -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \
         -e "/encrypted: yes/d" \
         -e "/cluster_size: [0-9]\\+/d" \
         -e "/table_size: [0-9]\\+/d" \
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 7d4781d..62529ee 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -85,6 +85,9 @@ else
     elif [ "$IMGPROTO" = "nfs" ]; then
         TEST_DIR="nfs://127.0.0.1/$TEST_DIR"
         TEST_IMG=$TEST_DIR/t.$IMGFMT
+    elif [ "$IMGPROTO" = "vxhs" ]; then
+        TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
+        TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT"
     else
         TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
     fi
@@ -171,6 +174,12 @@ _make_test_img()
         eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  $TEST_IMG_FILE >/dev/null &"
         sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
     fi
+
+    # Start QNIO server on image directory for vxhs protocol
+    if [ $IMGPROTO = "vxhs" ]; then
+        eval "$QEMU_VXHS -d  $TEST_DIR > /dev/null &"
+        sleep 1 # Wait for server to come up.
+    fi
 }
 
 _rm_test_img()
@@ -197,6 +206,16 @@ _cleanup_test_img()
             fi
             rm -f "$TEST_IMG_FILE"
             ;;
+        vxhs)
+            if [ -f "${TEST_DIR}/qemu-vxhs.pid" ]; then
+                local QEMU_VXHS_PID
+                read QEMU_VXHS_PID < "${TEST_DIR}/qemu-vxhs.pid"
+                kill ${QEMU_VXHS_PID} >/dev/null 2>&1
+                rm -f "${TEST_DIR}/qemu-vxhs.pid"
+            fi
+            rm -f "$TEST_IMG_FILE"
+            ;;
+
         file)
             _rm_test_img "$TEST_DIR/t.$IMGFMT"
             _rm_test_img "$TEST_DIR/t.$IMGFMT.orig"
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
@ 2017-04-04 15:55   ` Daniel P. Berrange
  2017-04-11 19:47   ` Jeff Cody
  2017-04-19 16:27   ` Stefan Hajnoczi
  2 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2017-04-04 15:55 UTC (permalink / raw)
  To: Ashish Mittal
  Cc: qemu-devel, pbonzini, kwolf, armbru, jcody, famz, ashish.mittal,
	stefanha, Ketan.Nilangekar, jferlan, Buddhi.Madhav, Suraj.Singh,
	Nitin.Jerath, peter.maydell, venkatesha.mg, Rakesh.Ranjan,
	eblake, Abhijit.Dey

On Mon, Apr 03, 2017 at 08:48:08PM -0700, Ashish Mittal wrote:
> Source code for the qnio library that this code loads can be downloaded from:
> https://github.com/VeritasHyperScale/libqnio.git
> 
> Sample command line using JSON syntax:
> ./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
> -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> -msg timestamp=on
> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
> "server":{"host":"172.172.17.4","port":"9999"}}'
> 
> Sample command line using URI syntax:
> qemu-img convert -f raw -O raw -n
> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
> vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0
> 
> Sample command line using TLS credentials (run in secure mode):
> ./qemu-io --object
> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
> 
> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>

I've not reviewed the i/o related stuff, but from the POV of TLS cred
handling / QAPI / disk open, this all looks fine to me now.

  Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-04  3:48 [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Ashish Mittal
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs" Ashish Mittal
@ 2017-04-04 18:02 ` Jeff Cody
  2017-04-20 15:03 ` Jeff Cody
  3 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-04 18:02 UTC (permalink / raw)
  To: Ashish Mittal
  Cc: qemu-devel, pbonzini, kwolf, armbru, berrange, famz,
	ashish.mittal, stefanha, Ketan.Nilangekar, jferlan,
	Buddhi.Madhav, Suraj.Singh, Nitin.Jerath, peter.maydell,
	venkatesha.mg, Rakesh.Ranjan, eblake, Abhijit.Dey

On Mon, Apr 03, 2017 at 08:48:07PM -0700, Ashish Mittal wrote:
> - Veritas HyperScale block driver in QEMU is designed to provide an accelerated
>   IO path from KVM virtual machines to Veritas HyperScale storage service.
> 
> - A network IO transfer library that translates block IO from HyperScale block
>   driver to a network IO format to send it to Veritas HyperScale storage
>   service. This library (libvxhs) has been open sourced and is available on
>   github here:  https://github.com/VeritasHyperScale/libqnio.git
> 
> Ashish Mittal (2):
>   block/vxhs.c: Add support for a new block device type called "vxhs"
>   block/vxhs.c: Add qemu-iotests for new block device type "vxhs"
> 
>  block/Makefile.objs              |   2 +
>  block/trace-events               |  17 ++
>  block/vxhs.c                     | 575 +++++++++++++++++++++++++++++++++++++++
>  configure                        |  39 +++
>  qapi/block-core.json             |  23 +-
>  tests/qemu-iotests/common        |   6 +
>  tests/qemu-iotests/common.config |  13 +
>  tests/qemu-iotests/common.filter |   1 +
>  tests/qemu-iotests/common.rc     |  19 ++
>  9 files changed, 693 insertions(+), 2 deletions(-)
>  create mode 100644 block/vxhs.c
> 
> -- 
> 2.5.5
>

With this additional patch (below), this passes all qemu-iotests for qcow2.


>From aeea8e23e28de325c572f8f3f67a1651b62887bd Mon Sep 17 00:00:00 2001
Message-Id: <aeea8e23e28de325c572f8f3f67a1651b62887bd.1491328865.git.jcody@redhat.com>
From: Jeff Cody <jcody@redhat.com>
Date: Tue, 14 Feb 2017 09:51:42 -0500
Subject: [PATCH v10 3/2 1/1] qemu-iotests: exclude vxhs from image creation
 via protocol

The protocol VXHS does not support image creation.  Some tests expect
to be able to create images through the protocol.  Exclude VXHS from
these tests.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/017 | 1 +
 tests/qemu-iotests/020 | 1 +
 tests/qemu-iotests/029 | 1 +
 tests/qemu-iotests/073 | 1 +
 tests/qemu-iotests/114 | 1 +
 tests/qemu-iotests/130 | 1 +
 tests/qemu-iotests/134 | 1 +
 tests/qemu-iotests/156 | 1 +
 tests/qemu-iotests/158 | 1 +
 9 files changed, 9 insertions(+)

diff --git a/tests/qemu-iotests/017 b/tests/qemu-iotests/017
index e3f9e75..4f9302d 100755
--- a/tests/qemu-iotests/017
+++ b/tests/qemu-iotests/017
@@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Any format supporting backing files
 _supported_fmt qcow qcow2 vmdk qed
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 _unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
 
diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
index 9c4a68c..7a11110 100755
--- a/tests/qemu-iotests/020
+++ b/tests/qemu-iotests/020
@@ -43,6 +43,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Any format supporting backing files
 _supported_fmt qcow qcow2 vmdk qed
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 _unsupported_imgopts "subformat=monolithicFlat" \
                      "subformat=twoGbMaxExtentFlat" \
diff --git a/tests/qemu-iotests/029 b/tests/qemu-iotests/029
index e639ac0..30bab24 100755
--- a/tests/qemu-iotests/029
+++ b/tests/qemu-iotests/029
@@ -42,6 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Any format supporting intenal snapshots
 _supported_fmt qcow2
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 # Internal snapshots are (currently) impossible with refcount_bits=1
 _unsupported_imgopts 'refcount_bits=1[^0-9]'
diff --git a/tests/qemu-iotests/073 b/tests/qemu-iotests/073
index ad37a61..40f85b1 100755
--- a/tests/qemu-iotests/073
+++ b/tests/qemu-iotests/073
@@ -39,6 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 
 CLUSTER_SIZE=64k
diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114
index f110d4f..5b7dc54 100755
--- a/tests/qemu-iotests/114
+++ b/tests/qemu-iotests/114
@@ -39,6 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 
 
diff --git a/tests/qemu-iotests/130 b/tests/qemu-iotests/130
index ecc8a5b..f941fc9 100755
--- a/tests/qemu-iotests/130
+++ b/tests/qemu-iotests/130
@@ -42,6 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 
 qemu_comm_method="monitor"
diff --git a/tests/qemu-iotests/134 b/tests/qemu-iotests/134
index af618b8..acce946 100755
--- a/tests/qemu-iotests/134
+++ b/tests/qemu-iotests/134
@@ -39,6 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 
 
diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156
index cc95ff1..78deaff 100755
--- a/tests/qemu-iotests/156
+++ b/tests/qemu-iotests/156
@@ -48,6 +48,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2 qed
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 
 # Create source disk
diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158
index a6cdd6d..ef8d70f 100755
--- a/tests/qemu-iotests/158
+++ b/tests/qemu-iotests/158
@@ -39,6 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto generic
+_unsupported_proto vxhs
 _supported_os Linux
 
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
  2017-04-04 15:55   ` Daniel P. Berrange
@ 2017-04-11 19:47   ` Jeff Cody
  2017-04-12  8:10     ` ashish mittal
  2017-04-19 16:27   ` Stefan Hajnoczi
  2 siblings, 1 reply; 12+ messages in thread
From: Jeff Cody @ 2017-04-11 19:47 UTC (permalink / raw)
  To: Ashish Mittal
  Cc: qemu-devel, pbonzini, kwolf, armbru, berrange, famz,
	ashish.mittal, stefanha, Ketan.Nilangekar, jferlan,
	Buddhi.Madhav, Suraj.Singh, Nitin.Jerath, peter.maydell,
	venkatesha.mg, Rakesh.Ranjan, eblake, Abhijit.Dey

On Mon, Apr 03, 2017 at 08:48:08PM -0700, Ashish Mittal wrote:
> Source code for the qnio library that this code loads can be downloaded from:
> https://github.com/VeritasHyperScale/libqnio.git
> 
> Sample command line using JSON syntax:
> ./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
> -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> -msg timestamp=on
> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
> "server":{"host":"172.172.17.4","port":"9999"}}'
> 
> Sample command line using URI syntax:
> qemu-img convert -f raw -O raw -n
> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
> vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0
> 
> Sample command line using TLS credentials (run in secure mode):
> ./qemu-io --object
> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
> 
> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>

I was testing this some with blockdev-add and blockdev-del, and this
sequence causes a segfault:

1. blockdev-add vxhs image
2. blockdev-del above image
3. blockdev-add vxhs image  <--- segfaults

Looking at it in gdb, this is an issue with libqnio.  The call to iio_fini()
is not sufficiently thorough in cleaning up resources.

In nio_client.c, qnc_ctx is never freed, because there does not
seem to be a call such as 'qnc_driver_fini' that cleans up the allocated
qnio_client_ctx.

Therefore, on the second call to iio_init, the libqnio internal variable
network_driver is NULL, because qnc_driver_init() returns NULL if it is
called when qnc_ctx is still initialized:



lib/qnio/nio_client.c:

411 int
412 iio_init(int32_t version, iio_cb_t cb)
413 {

[...]

432     apictx->network_driver = qnc_secure_driver_init(client_callback);
433     nioDbg("Created API context.\n");
434     return 0;
435 } 

[...]

779 struct channel_driver *
780 qnc_driver_init(qnio_notify client_notify)
781 {
782     if (qnc_ctx) {
783         nioDbg("Driver already initialized");
784         return NULL;
785     }
786


So two issues:

A. iio_init() should check the returned pointer, and fail if NULL

B. iio_fini() needs to clean everything up so that a new vxhs connection is
   possible. This likely means at least one new function in nio_client.c to
   clean up qnc_ctx.

-Jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-11 19:47   ` Jeff Cody
@ 2017-04-12  8:10     ` ashish mittal
  2017-04-12 12:15       ` Jeff Cody
  0 siblings, 1 reply; 12+ messages in thread
From: ashish mittal @ 2017-04-12  8:10 UTC (permalink / raw)
  To: Jeff Cody
  Cc: qemu-devel, Paolo Bonzini, Kevin Wolf, Markus Armbruster,
	Daniel P. Berrange, Fam Zheng, Ashish Mittal, Stefan Hajnoczi,
	Ketan Nilangekar, John Ferlan, Buddhi Madhav, Suraj Singh,
	Nitin Jerath, Peter Maydell, Venkatesha M.G.,
	Rakesh Ranjan, Eric Blake, Abhijit Dey

On Tue, Apr 11, 2017 at 12:47 PM, Jeff Cody <jcody@redhat.com> wrote:
> On Mon, Apr 03, 2017 at 08:48:08PM -0700, Ashish Mittal wrote:
>> Source code for the qnio library that this code loads can be downloaded from:
>> https://github.com/VeritasHyperScale/libqnio.git
>>
>> Sample command line using JSON syntax:
>> ./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
>> -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
>> -msg timestamp=on
>> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
>> "server":{"host":"172.172.17.4","port":"9999"}}'
>>
>> Sample command line using URI syntax:
>> qemu-img convert -f raw -O raw -n
>> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
>> vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0
>>
>> Sample command line using TLS credentials (run in secure mode):
>> ./qemu-io --object
>> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
>> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
>> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
>>
>> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
>
> I was testing this some with blockdev-add and blockdev-del, and this
> sequence causes a segfault:
>
> 1. blockdev-add vxhs image
> 2. blockdev-del above image
> 3. blockdev-add vxhs image  <--- segfaults
>
> Looking at it in gdb, this is an issue with libqnio.  The call to iio_fini()
> is not sufficiently thorough in cleaning up resources.
>
> In nio_client.c, qnc_ctx is never freed, because there does not
> seem to be a call such as 'qnc_driver_fini' that cleans up the allocated
> qnio_client_ctx.
>
> Therefore, on the second call to iio_init, the libqnio internal variable
> network_driver is NULL, because qnc_driver_init() returns NULL if it is
> called when qnc_ctx is still initialized:
>
>
>
> lib/qnio/nio_client.c:
>
> 411 int
> 412 iio_init(int32_t version, iio_cb_t cb)
> 413 {
>
> [...]
>
> 432     apictx->network_driver = qnc_secure_driver_init(client_callback);
> 433     nioDbg("Created API context.\n");
> 434     return 0;
> 435 }
>
> [...]
>
> 779 struct channel_driver *
> 780 qnc_driver_init(qnio_notify client_notify)
> 781 {
> 782     if (qnc_ctx) {
> 783         nioDbg("Driver already initialized");
> 784         return NULL;
> 785     }
> 786
>
>
> So two issues:
>
> A. iio_init() should check the returned pointer, and fail if NULL
>
> B. iio_fini() needs to clean everything up so that a new vxhs connection is
>    possible. This likely means at least one new function in nio_client.c to
>    clean up qnc_ctx.
>
> -Jeff

Thanks for reporting the issue and also suggesting the fix.
I was able to reproduce the issue and have checked in a fix to libqnio.

This was the stack trace from the core -

Program terminated with signal 11, Segmentation fault.
#0  0x00007f12083a3afa in iio_channel_open (uri=0x7f120c57eb70
"of://127.0.0.1:9999", cacert=0x0, client_key=0x0, client_cert=0x0) at
lib/qnio/iioapi.c:105
#1  0x00007f12083a470a in iio_open (uri=0x7f120d121be0
"of://127.0.0.1:9999", devid=0x7f120baa4bb0 "/test.raw", flags=0,
cacert=0x0, client_key=0x0, client_cert=0x0) at lib/qnio/iioapi.c:520
#2  0x00007f12091bf7a3 in vxhs_open (bs=<optimized out>,
options=<optimized out>, bdrv_flags=<optimized out>,
errp=0x7ffdf1cafd70) at block/vxhs.c:388
#3  0x00007f120916cf14 in bdrv_open_driver
(bs=bs@entry=0x7f120b1481b0, drv=drv@entry=0x7f1209843580 <bdrv_vxhs>,
node_name=<optimized out>, options=options@entry=0x7f120daa6840,
open_flags=8194,
    errp=errp@entry=0x7ffdf1cafe28) at block.c:1011
#4  0x00007f120917079f in bdrv_open_common (errp=0x7ffdf1cafe28,
options=0x7f120daa6840, file=0x0, bs=0x7f120b1481b0) at block.c:1250
#5  bdrv_open_inherit (filename=<optimized out>, filename@entry=0x0,
reference=reference@entry=0x0, options=0x7f120daa6840,
options@entry=0x7f120daa4800, flags=40962, parent=parent@entry=0x0,
    child_role=child_role@entry=0x0, errp=errp@entry=0x7ffdf1caff18)
at block.c:2413
#6  0x00007f1209171663 in bdrv_open (filename=filename@entry=0x0,
reference=reference@entry=0x0, options=options@entry=0x7f120daa4800,
flags=<optimized out>, errp=errp@entry=0x7ffdf1caff18)
    at block.c:2505
#7  0x00007f1208f9f026 in bds_tree_init
(bs_opts=bs_opts@entry=0x7f120daa4800, errp=errp@entry=0x7ffdf1caff18)
at blockdev.c:656
#8  0x00007f1208fa4e5b in qmp_blockdev_add
(options=options@entry=0x7ffdf1caff20, errp=errp@entry=0x7ffdf1caff18)
at blockdev.c:3885
...

Please test again with refreshed libqnio and let me know in case of issues.

Thanks,
Ashish

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-12  8:10     ` ashish mittal
@ 2017-04-12 12:15       ` Jeff Cody
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-12 12:15 UTC (permalink / raw)
  To: ashish mittal
  Cc: qemu-devel, Paolo Bonzini, Kevin Wolf, Markus Armbruster,
	Daniel P. Berrange, Fam Zheng, Ashish Mittal, Stefan Hajnoczi,
	Ketan Nilangekar, John Ferlan, Buddhi Madhav, Suraj Singh,
	Nitin Jerath, Peter Maydell, Venkatesha M.G.,
	Rakesh Ranjan, Eric Blake, Abhijit Dey

On Wed, Apr 12, 2017 at 01:10:10AM -0700, ashish mittal wrote:
> On Tue, Apr 11, 2017 at 12:47 PM, Jeff Cody <jcody@redhat.com> wrote:
> > On Mon, Apr 03, 2017 at 08:48:08PM -0700, Ashish Mittal wrote:
> >> Source code for the qnio library that this code loads can be downloaded from:
> >> https://github.com/VeritasHyperScale/libqnio.git
> >>
> >> Sample command line using JSON syntax:
> >> ./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
> >> -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> >> -msg timestamp=on
> >> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
> >> "server":{"host":"172.172.17.4","port":"9999"}}'
> >>
> >> Sample command line using URI syntax:
> >> qemu-img convert -f raw -O raw -n
> >> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
> >> vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0
> >>
> >> Sample command line using TLS credentials (run in secure mode):
> >> ./qemu-io --object
> >> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
> >> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
> >> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
> >>
> >> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
> >
> > I was testing this some with blockdev-add and blockdev-del, and this
> > sequence causes a segfault:
> >
> > 1. blockdev-add vxhs image
> > 2. blockdev-del above image
> > 3. blockdev-add vxhs image  <--- segfaults
> >
> > Looking at it in gdb, this is an issue with libqnio.  The call to iio_fini()
> > is not sufficiently thorough in cleaning up resources.
> >
> > In nio_client.c, qnc_ctx is never freed, because there does not
> > seem to be a call such as 'qnc_driver_fini' that cleans up the allocated
> > qnio_client_ctx.
> >
> > Therefore, on the second call to iio_init, the libqnio internal variable
> > network_driver is NULL, because qnc_driver_init() returns NULL if it is
> > called when qnc_ctx is still initialized:
> >
> >
> >
> > lib/qnio/nio_client.c:
> >
> > 411 int
> > 412 iio_init(int32_t version, iio_cb_t cb)
> > 413 {
> >
> > [...]
> >
> > 432     apictx->network_driver = qnc_secure_driver_init(client_callback);
> > 433     nioDbg("Created API context.\n");
> > 434     return 0;
> > 435 }
> >
> > [...]
> >
> > 779 struct channel_driver *
> > 780 qnc_driver_init(qnio_notify client_notify)
> > 781 {
> > 782     if (qnc_ctx) {
> > 783         nioDbg("Driver already initialized");
> > 784         return NULL;
> > 785     }
> > 786
> >
> >
> > So two issues:
> >
> > A. iio_init() should check the returned pointer, and fail if NULL
> >
> > B. iio_fini() needs to clean everything up so that a new vxhs connection is
> >    possible. This likely means at least one new function in nio_client.c to
> >    clean up qnc_ctx.
> >
> > -Jeff
> 
> Thanks for reporting the issue and also suggesting the fix.
> I was able to reproduce the issue and have checked in a fix to libqnio.
> 
> This was the stack trace from the core -
> 
> Program terminated with signal 11, Segmentation fault.
> #0  0x00007f12083a3afa in iio_channel_open (uri=0x7f120c57eb70
> "of://127.0.0.1:9999", cacert=0x0, client_key=0x0, client_cert=0x0) at
> lib/qnio/iioapi.c:105
> #1  0x00007f12083a470a in iio_open (uri=0x7f120d121be0
> "of://127.0.0.1:9999", devid=0x7f120baa4bb0 "/test.raw", flags=0,
> cacert=0x0, client_key=0x0, client_cert=0x0) at lib/qnio/iioapi.c:520
> #2  0x00007f12091bf7a3 in vxhs_open (bs=<optimized out>,
> options=<optimized out>, bdrv_flags=<optimized out>,
> errp=0x7ffdf1cafd70) at block/vxhs.c:388
> #3  0x00007f120916cf14 in bdrv_open_driver
> (bs=bs@entry=0x7f120b1481b0, drv=drv@entry=0x7f1209843580 <bdrv_vxhs>,
> node_name=<optimized out>, options=options@entry=0x7f120daa6840,
> open_flags=8194,
>     errp=errp@entry=0x7ffdf1cafe28) at block.c:1011
> #4  0x00007f120917079f in bdrv_open_common (errp=0x7ffdf1cafe28,
> options=0x7f120daa6840, file=0x0, bs=0x7f120b1481b0) at block.c:1250
> #5  bdrv_open_inherit (filename=<optimized out>, filename@entry=0x0,
> reference=reference@entry=0x0, options=0x7f120daa6840,
> options@entry=0x7f120daa4800, flags=40962, parent=parent@entry=0x0,
>     child_role=child_role@entry=0x0, errp=errp@entry=0x7ffdf1caff18)
> at block.c:2413
> #6  0x00007f1209171663 in bdrv_open (filename=filename@entry=0x0,
> reference=reference@entry=0x0, options=options@entry=0x7f120daa4800,
> flags=<optimized out>, errp=errp@entry=0x7ffdf1caff18)
>     at block.c:2505
> #7  0x00007f1208f9f026 in bds_tree_init
> (bs_opts=bs_opts@entry=0x7f120daa4800, errp=errp@entry=0x7ffdf1caff18)
> at blockdev.c:656
> #8  0x00007f1208fa4e5b in qmp_blockdev_add
> (options=options@entry=0x7ffdf1caff20, errp=errp@entry=0x7ffdf1caff18)
> at blockdev.c:3885
> ...
> 
> Please test again with refreshed libqnio and let me know in case of issues.
>

Hi Ashish,

I updated libqnio to 87b641b, and I can confirm that it fixes the issue I
was seeing above.

Thanks,
Jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
  2017-04-04 15:55   ` Daniel P. Berrange
  2017-04-11 19:47   ` Jeff Cody
@ 2017-04-19 16:27   ` Stefan Hajnoczi
  2017-04-20  1:01     ` ashish mittal
  2 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-04-19 16:27 UTC (permalink / raw)
  To: Ashish Mittal
  Cc: qemu-devel, pbonzini, kwolf, armbru, berrange, jcody, famz,
	ashish.mittal, Ketan.Nilangekar, jferlan, Buddhi.Madhav,
	Suraj.Singh, Nitin.Jerath, peter.maydell, venkatesha.mg,
	Rakesh.Ranjan, eblake, Abhijit.Dey

[-- Attachment #1: Type: text/plain, Size: 2151 bytes --]

On Mon, Apr 03, 2017 at 08:48:08PM -0700, Ashish Mittal wrote:
> Source code for the qnio library that this code loads can be downloaded from:
> https://github.com/VeritasHyperScale/libqnio.git
> 
> Sample command line using JSON syntax:
> ./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
> -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> -msg timestamp=on
> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
> "server":{"host":"172.172.17.4","port":"9999"}}'
> 
> Sample command line using URI syntax:
> qemu-img convert -f raw -O raw -n
> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
> vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0
> 
> Sample command line using TLS credentials (run in secure mode):
> ./qemu-io --object
> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
> 
> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
> ---
> v11 changelog:
> (1) Replaced InetSocketAddress with InetSocketAddressBase.
> (2) Removed access to qemu_uuid.
> (3) Removed unnecessary g_strdup()/g_free().
> (4) Removed unused acb->qiov.
> (5) Changed vxhs_init_and_ref() and vxhs_unref() per suggestion.
> (6) Removed unnecessary initializations from local variables.

QEMU code:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

libvxhs is not robust yet.  Here are two examples:

1. If the response from the server has an invalid magic number then the
   whole VM is aborted:

   QNIO_API_(void) kvset_unmarshal(qnio_byte_t * bs, kvset_t * *p)
   {
       ...
       assert(magic == kvset_magic);

2. There are buffer overflows and other memory corruptions.  For example
   when kv_binary_unpack() gets size=-1 over the wire.

The code needs to be audited line-by-line by someone aware of secure
coding practices.  Please look into this.

Also, is there a plan for getting libvxhs into Fedora and Debian?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-19 16:27   ` Stefan Hajnoczi
@ 2017-04-20  1:01     ` ashish mittal
  0 siblings, 0 replies; 12+ messages in thread
From: ashish mittal @ 2017-04-20  1:01 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, Paolo Bonzini, Kevin Wolf, Markus Armbruster,
	Daniel P. Berrange, Jeff Cody, Fam Zheng, Ashish Mittal,
	Ketan Nilangekar, John Ferlan, Buddhi Madhav, Suraj Singh,
	Nitin Jerath, Peter Maydell, Venkatesha M.G.,
	Rakesh Ranjan, Eric Blake, Abhijit Dey

On Wed, Apr 19, 2017 at 9:27 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Apr 03, 2017 at 08:48:08PM -0700, Ashish Mittal wrote:
>> Source code for the qnio library that this code loads can be downloaded from:
>> https://github.com/VeritasHyperScale/libqnio.git
>>
>> Sample command line using JSON syntax:
>> ./x86_64-softmmu/qemu-system-x86_64 -name instance-00000008 -S -vnc 0.0.0.0:0
>> -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
>> -msg timestamp=on
>> 'json:{"driver":"vxhs","vdisk-id":"c3e9095a-a5ee-4dce-afeb-2a59fb387410",
>> "server":{"host":"172.172.17.4","port":"9999"}}'
>>
>> Sample command line using URI syntax:
>> qemu-img convert -f raw -O raw -n
>> /var/lib/nova/instances/_base/0c5eacd5ebea5ed914b6a3e7b18f1ce734c386ad
>> vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0
>>
>> Sample command line using TLS credentials (run in secure mode):
>> ./qemu-io --object
>> tls-creds-x509,id=tls0,dir=/etc/pki/qemu/vxhs,endpoint=client -c 'read
>> -v 66000 2.5k' 'json:{"server.host": "127.0.0.1", "server.port": "9999",
>> "vdisk-id": "/test.raw", "driver": "vxhs", "tls-creds":"tls0"}'
>>
>> Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com>
>> ---
>> v11 changelog:
>> (1) Replaced InetSocketAddress with InetSocketAddressBase.
>> (2) Removed access to qemu_uuid.
>> (3) Removed unnecessary g_strdup()/g_free().
>> (4) Removed unused acb->qiov.
>> (5) Changed vxhs_init_and_ref() and vxhs_unref() per suggestion.
>> (6) Removed unnecessary initializations from local variables.
>
> QEMU code:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
Thanks!

> libvxhs is not robust yet.  Here are two examples:
>
> 1. If the response from the server has an invalid magic number then the
>    whole VM is aborted:
>
>    QNIO_API_(void) kvset_unmarshal(qnio_byte_t * bs, kvset_t * *p)
>    {
>        ...
>        assert(magic == kvset_magic);
>
> 2. There are buffer overflows and other memory corruptions.  For example
>    when kv_binary_unpack() gets size=-1 over the wire.
>

We will continue to audit the library code and fix potential security
issues with libvxhs.

> The code needs to be audited line-by-line by someone aware of secure
> coding practices.  Please look into this.
>
> Also, is there a plan for getting libvxhs into Fedora and Debian?

At the moment, our requirement is only RHEL. The library should build
and work fine with any distro.
We also plan to build source tarballs and checkin to github for every
release of libvxhs.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-04  3:48 [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Ashish Mittal
                   ` (2 preceding siblings ...)
  2017-04-04 18:02 ` [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Jeff Cody
@ 2017-04-20 15:03 ` Jeff Cody
  2017-04-20 16:03   ` ashish mittal
  3 siblings, 1 reply; 12+ messages in thread
From: Jeff Cody @ 2017-04-20 15:03 UTC (permalink / raw)
  To: Ashish Mittal
  Cc: qemu-devel, pbonzini, kwolf, armbru, berrange, famz,
	ashish.mittal, stefanha, Ketan.Nilangekar, jferlan,
	Buddhi.Madhav, Suraj.Singh, Nitin.Jerath, peter.maydell,
	venkatesha.mg, Rakesh.Ranjan, eblake, Abhijit.Dey

On Mon, Apr 03, 2017 at 08:48:07PM -0700, Ashish Mittal wrote:
> - Veritas HyperScale block driver in QEMU is designed to provide an accelerated
>   IO path from KVM virtual machines to Veritas HyperScale storage service.
> 
> - A network IO transfer library that translates block IO from HyperScale block
>   driver to a network IO format to send it to Veritas HyperScale storage
>   service. This library (libvxhs) has been open sourced and is available on
>   github here:  https://github.com/VeritasHyperScale/libqnio.git
> 
> Ashish Mittal (2):
>   block/vxhs.c: Add support for a new block device type called "vxhs"
>   block/vxhs.c: Add qemu-iotests for new block device type "vxhs"
> 
>  block/Makefile.objs              |   2 +
>  block/trace-events               |  17 ++
>  block/vxhs.c                     | 575 +++++++++++++++++++++++++++++++++++++++
>  configure                        |  39 +++
>  qapi/block-core.json             |  23 +-
>  tests/qemu-iotests/common        |   6 +
>  tests/qemu-iotests/common.config |  13 +
>  tests/qemu-iotests/common.filter |   1 +
>  tests/qemu-iotests/common.rc     |  19 ++
>  9 files changed, 693 insertions(+), 2 deletions(-)
>  create mode 100644 block/vxhs.c
> 
> -- 
> 2.5.5
> 

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs"
  2017-04-20 15:03 ` Jeff Cody
@ 2017-04-20 16:03   ` ashish mittal
  0 siblings, 0 replies; 12+ messages in thread
From: ashish mittal @ 2017-04-20 16:03 UTC (permalink / raw)
  To: Jeff Cody
  Cc: qemu-devel, Paolo Bonzini, Kevin Wolf, Markus Armbruster,
	Daniel P. Berrange, Fam Zheng, Ashish Mittal, Stefan Hajnoczi,
	Ketan Nilangekar, John Ferlan, Buddhi Madhav, Suraj Singh,
	Nitin Jerath, Peter Maydell, Venkatesha M.G.,
	Rakesh Ranjan, Eric Blake, Abhijit Dey

Thanks Jeff!

On Thu, Apr 20, 2017 at 8:03 AM, Jeff Cody <jcody@redhat.com> wrote:
> On Mon, Apr 03, 2017 at 08:48:07PM -0700, Ashish Mittal wrote:
>> - Veritas HyperScale block driver in QEMU is designed to provide an accelerated
>>   IO path from KVM virtual machines to Veritas HyperScale storage service.
>>
>> - A network IO transfer library that translates block IO from HyperScale block
>>   driver to a network IO format to send it to Veritas HyperScale storage
>>   service. This library (libvxhs) has been open sourced and is available on
>>   github here:  https://github.com/VeritasHyperScale/libqnio.git
>>
>> Ashish Mittal (2):
>>   block/vxhs.c: Add support for a new block device type called "vxhs"
>>   block/vxhs.c: Add qemu-iotests for new block device type "vxhs"
>>
>>  block/Makefile.objs              |   2 +
>>  block/trace-events               |  17 ++
>>  block/vxhs.c                     | 575 +++++++++++++++++++++++++++++++++++++++
>>  configure                        |  39 +++
>>  qapi/block-core.json             |  23 +-
>>  tests/qemu-iotests/common        |   6 +
>>  tests/qemu-iotests/common.config |  13 +
>>  tests/qemu-iotests/common.filter |   1 +
>>  tests/qemu-iotests/common.rc     |  19 ++
>>  9 files changed, 693 insertions(+), 2 deletions(-)
>>  create mode 100644 block/vxhs.c
>>
>> --
>> 2.5.5
>>
>
> Thanks,
>
> Applied to my block branch:
>
> git://github.com/codyprime/qemu-kvm-jtc block
>
> -Jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-04-20 16:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-04  3:48 [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Ashish Mittal
2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 1/2] " Ashish Mittal
2017-04-04 15:55   ` Daniel P. Berrange
2017-04-11 19:47   ` Jeff Cody
2017-04-12  8:10     ` ashish mittal
2017-04-12 12:15       ` Jeff Cody
2017-04-19 16:27   ` Stefan Hajnoczi
2017-04-20  1:01     ` ashish mittal
2017-04-04  3:48 ` [Qemu-devel] [PATCH v11 2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs" Ashish Mittal
2017-04-04 18:02 ` [Qemu-devel] [PATCH v11 0/2] block/vxhs.c: Add support for a new block device type called "vxhs" Jeff Cody
2017-04-20 15:03 ` Jeff Cody
2017-04-20 16:03   ` ashish mittal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.