$SUBJ
s/Change to support/Add support for
s/ with qemu-kvm//
On 11/12/2016 01:19 AM, Ashish Mittal wrote:
> Sample XML for a vxhs vdisk is as follows:
>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw' cache='none'/>
> <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251'>
> <host name='192.168.0.1' port='9999'/>
> </source>
> <backingStore/>
> <target dev='vda' bus='virtio'/>
> <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
> <alias name='virtio-disk0'/>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
> </disk>
>
> Signed-off-by: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx>
> ---
> docs/formatdomain.html.in | 12 ++++++--
> docs/schemas/domaincommon.rng | 1 +
> src/qemu/qemu_command.c | 4 +++
> src/qemu/qemu_driver.c | 3 ++
> src/qemu/qemu_parse_command.c | 25 ++++++++++++++++
> src/util/virstoragefile.c | 4 ++-
> src/util/virstoragefile.h | 1 +
> .../qemuxml2argv-disk-drive-network-vxhs.args | 23 +++++++++++++++
> .../qemuxml2argv-disk-drive-network-vxhs.xml | 34 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 1 +
> 10 files changed, 104 insertions(+), 4 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml
>
Missing changes for src/libxl/libxl_conf.c (libxlMakeNetworkDiskSrcStr)
and src/xenconfig/xen_xl.c (xenFormatXLDiskSrcNet) - need to add a "case
VIR_STORAGE_NET_PROTOCOL_VXHS:" for the "switch (virStorageNetProtocol)
src->protocol".
Also running 'make syntax-check' would show that
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args has a
too long line at the end:
GEN test-wrap-argv
--- tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
2016-11-14 06:16:40.739509847 -0500
+++ - 2016-11-14 06:23:59.661334157 -0500
@@ -20,4 +20,5 @@
-usb \
-drive file=vxhs://192.168.0.1:9999/eb90327c-8302-4725-9e1b-4e85ed4dc251,\
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
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
+id=virtio-disk0
Incorrect line wrapping in
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
Use test-wrap-argv.pl to wrap test data files
cfg.mk:1075: recipe for target 'test-wrap-argv' failed
make: *** [test-wrap-argv] Error 1
All those are easily fixable; however, you will also need to modify the
virStorageSourceJSONDriverParser in src/util/virstoragefile.c in order
to do the JSON parsing properly as well as tests/virstoragetest.c to add
test case(s). If you use 'gitk' or some other git log -p browser,
starting with commit id 'e91f767c743' and going forward through Peter's
series you can see how this was done for other various protocols.
>From what I see from the QEMU list that'll work for the json you
provided in your example:
Sample command line using the JSON syntax:
./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"}}'
Other questions/concerns... These may have been answered previously, but
I haven't followed closely along.
1. It appears to me that the 'name' field is a UUID for the VxHS
volume, but it's not clear that's all it can be. You'll note in
virDomainDiskSourceParse a specific gluster check. So that we don't get
surprised at some point in the future - are there any specific things in
the name that need to be addressed?
Beyond that, how does one know what to provide for that 'name=' field?
For VxHS, it seems to me that the path must be a UUID of some sort. So
would anyone know which UUID to magically pick in order to add the disk
to a guest? How does one get a list?
2. Does/will VxHS ever require authentication similar to iSCSI and RBD?
3. Will there be (or is there) support for LUKS volumes on the VxHS server?
4. There's no VxHS Storage Pool support in this patch (OK actually an
additional set of patches in order to support). That would be expected
especially for a networked storage environment. You'll note there are
src/storage/storage_backend_{iscsi|gluster|rbd}.{c|h} files that manage
iSCSI, Gluster, and RBD protocol specific things. For starters, create,
delete, and refresh - especially things that a stat() wouldn't
necessarily return (capacity, allocation, type a/k/a target.format).
Perhaps even the ability to upload/download and wipe volumes in the
pool. Having a pool is a bit more work, but look at the genesis of
existing storage_backend_*.c files to get a sense for what needs to change.
5. Along the same lines - virStorageFileBackendForTypeInternal handles
getting details/metadata about the file format in order to determine
block sizes for inactive domains. See the path qemuDomainGetBlockInfo
calling into qemuStorageLimitsRefresh which calls virStorageFileInitAs.
These would be for the backend of a 'virsh domblkinfo' type command when
the domain isn't running. The gluster backend has a set of them (search
on _virStorageFileBackend and virStorageFileBackendGluster).
6. Use something like cscope to be sure to check all the places where
VIR_STORAGE_NET_PROTOCOL_ is used in the code and made sure there isn't
a special case for VXHS (RBD, GLUSTER, and ISCSI have a few). Similarly
check usage of VIR_STORAGE_TYPE_NETWORK. Some checks are not within
switch/case statements and some involve specific disk configuration
options that are(n't) supported. What about any need for migration?
(see qemuMigrationIsSafe)
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 11b3330..ade35a3 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2276,9 +2276,9 @@
> <dd>
> The <code>protocol</code> attribute specifies the protocol to
> access to the requested image. Possible values are "nbd",
> - "iscsi", "rbd", "sheepdog" or "gluster". If the
> - <code>protocol</code> attribute is "rbd", "sheepdog" or
> - "gluster", an additional attribute <code>name</code> is
> + "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". If the
> + <code>protocol</code> attribute is "rbd", "sheepdog", "gluster"
> + or "vxhs", an additional attribute <code>name</code> is
> mandatory to specify which volume/image will be used. For "nbd",
> the <code>name</code> attribute is optional. For "iscsi"
> (<span class="since">since 1.0.4</span>), the <code>name</code>
Just before the closing </dd> add something to describe what the 'name'
will be for vxfs like is done for iscsi, such as:
For "vxhs" (<span class="since">since 2.5.0</span>), the
<code>name</code> is ... [it looks like a UUID to me - although there's
nothing that validates that hypothesis].
> @@ -2388,6 +2388,12 @@
> <td> one or more (<span class="since">Since 2.1.0</span>), just one prior to that </td>
> <td> 24007 </td>
> </tr>
> + <tr>
> + <td> vxhs </td>
> + <td> a server running Veritas HyperScale daemon </td>
> + <td> only one </td>
> + <td> 9999 </td>
> + </tr>
> </table>
> <p>
> gluster supports "tcp", "rdma", "unix" as valid values for the
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 19d45fd..e569e0a 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1464,6 +1464,7 @@
> <value>ftp</value>
> <value>ftps</value>
> <value>tftp</value>
> + <value>vxhs</value>
> </choice>
> </attribute>
> <optional>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4a5fce3..c7b95aa 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -491,6 +491,9 @@ qemuNetworkDriveGetPort(int protocol,
> /* no default port specified */
> return 0;
>
> + case VIR_STORAGE_NET_PROTOCOL_VXHS:
> + return 9999;
> +
> case VIR_STORAGE_NET_PROTOCOL_RBD:
> case VIR_STORAGE_NET_PROTOCOL_LAST:
> case VIR_STORAGE_NET_PROTOCOL_NONE:
> @@ -1034,6 +1037,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
> case VIR_STORAGE_NET_PROTOCOL_TFTP:
> case VIR_STORAGE_NET_PROTOCOL_ISCSI:
> case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
> + case VIR_STORAGE_NET_PROTOCOL_VXHS:
> ret = qemuBuildNetworkDriveURI(src, secinfo);
> break;
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a82e58b..4910004 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -13724,6 +13724,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
> 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:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("external inactive snapshots are not supported on "
> @@ -13787,6 +13788,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
> 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:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("external active snapshots are not supported on "
> @@ -13932,6 +13934,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn,
> 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:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("internal inactive snapshots are not supported on "
> diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
> index c3b27aa..f2edc28 100644
> --- a/src/qemu/qemu_parse_command.c
> +++ b/src/qemu/qemu_parse_command.c
> @@ -263,6 +263,16 @@ qemuParseNBDString(virDomainDiskDefPtr disk)
> return -1;
> }
>
> +static int
> +qemuParseVxHSString(virDomainDiskDefPtr def)
> +{
> + virURIPtr uri = NULL;
> +
> + if (!(uri = virURIParse(def->src->path)))
> + return -1;
> +
Should there be validation of the name? Something like a call to
virUUIDIsValid?
This is code for the path of parsing a qemu command line of a guest not
started by libvirt in order to add it to libvirt (virsh qemu-attach),
which is used "less often", so I'm less concerned about this, but mainly
curious.
John
> + return qemuParseDriveURIString(def, uri, "vxhs");
> +}
>
> /*
> * This method takes a string representing a QEMU command line ARGV set
> @@ -737,6 +747,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
> if (VIR_STRDUP(def->src->path, vdi) < 0)
> goto error;
> }
> + } else if (STRPREFIX(def->src->path, "vxhs:")) {
> + def->src->type = VIR_STORAGE_TYPE_NETWORK;
> + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS;
> +
> + if (qemuParseVxHSString(def) < 0)
> + goto error;
> } else {
> def->src->type = VIR_STORAGE_TYPE_FILE;
> }
> @@ -1933,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;
> }
> @@ -2009,6 +2029,11 @@ qemuParseCommandLine(virCapsPtr caps,
> goto error;
>
> break;
> + case VIR_STORAGE_NET_PROTOCOL_VXHS:
> + if (qemuParseVxHSString(disk) < 0)
> + goto error;
> +
> + break;
> case VIR_STORAGE_NET_PROTOCOL_HTTP:
> case VIR_STORAGE_NET_PROTOCOL_HTTPS:
> case VIR_STORAGE_NET_PROTOCOL_FTP:
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 272db67..a730a01 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -86,7 +86,8 @@ VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST,
> "ftp",
> "ftps",
> "tftp",
> - "ssh")
> + "ssh",
> + "vxhs")
>
> VIR_ENUM_IMPL(virStorageNetHostTransport, VIR_STORAGE_NET_HOST_TRANS_LAST,
> "tcp",
> @@ -2634,6 +2635,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src,
> case VIR_STORAGE_NET_PROTOCOL_ISCSI:
> case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
> case VIR_STORAGE_NET_PROTOCOL_SSH:
> + case VIR_STORAGE_NET_PROTOCOL_VXHS:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("malformed backing store path for protocol %s"),
> protocol);
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 3d09468..88dff36 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -132,6 +132,7 @@ typedef enum {
> VIR_STORAGE_NET_PROTOCOL_FTPS,
> VIR_STORAGE_NET_PROTOCOL_TFTP,
> VIR_STORAGE_NET_PROTOCOL_SSH,
> + VIR_STORAGE_NET_PROTOCOL_VXHS,
>
> VIR_STORAGE_NET_PROTOCOL_LAST
> } virStorageNetProtocol;
> 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..3d37b00
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
> @@ -0,0 +1,23 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/libexec/qemu-kvm \
> +-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 \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-drive file=vxhs://192.168.0.1:9999/eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> +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/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml
> new file mode 100644
> index 0000000..45c807f
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml
> @@ -0,0 +1,34 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/libexec/qemu-kvm</emulator>
> + <disk type='network' device='disk'>
> + <driver name='qemu' type='raw' cache='none'/>
> + <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251'>
> + <host name='192.168.0.1' port='9999'/>
> + </source>
> + <backingStore/>
> + <target dev='vda' bus='virtio'/>
> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
> + <alias name='virtio-disk0'/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
> + </disk>
> + <controller type='usb' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 1ee8402..d94e3f2 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -869,6 +869,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", NONE);
> DO_TEST("disk-drive-no-boot",
> QEMU_CAPS_BOOTINDEX);
> DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid",
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
