On 02/25/2013 10:44 AM, Paolo Bonzini wrote: A bit more context helps: char *host, *port; host = disk->src; disk->hosts->name = host; So there is definitely a use-after-free bug fixed by your patch. However, your patch causes a double-free bug on error (if the strdup(port) fails, then disk->hosts->port and disk->src are the same pointer, but we attempt to free both of them). I'm squashing this in before pushing, to make the transfer semantics more obvious: diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index 0a7d4ec..f8f3ade 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -8832,11 +8832,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, if (VIR_ALLOC(disk->hosts) < 0) goto no_memory; disk->nhosts = 1; - disk->hosts->name = host; + disk->hosts->name = disk->src; + disk->src = NULL; disk->hosts->port = strdup(port); if (!disk->hosts->port) goto no_memory; - disk->src = NULL; break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: /* old-style CEPH_ARGS env variable is parsed later */ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Il 28/02/2013 06:03, Eric Blake ha scritto: ACK Paolo

Move the code to an external function, and structure it to prepare the addition of new features in the next few patches. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- src/qemu/qemu_command.c | 128 ++++++++++++++++++++++++++++-------------------- tests/qemuxml2xmltest.c | 1 + 2 files changed, 76 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a3c5a4e..beb7cfe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2128,6 +2128,45 @@ error: } static int +qemuParseNBDString(virDomainDiskDefPtr disk) +{ + virDomainDiskHostDefPtr h = NULL; + char *host, *port; + + if (VIR_ALLOC(h) < 0) + goto no_memory; + + host = disk->src + strlen("nbd:"); + port = strchr(host, ':'); + if (!port) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), disk->src); + goto error; + } + + *port++ = '\0'; + h->name = strdup(host); + if (!h->name) + goto no_memory; + + h->port = strdup(port); + if (!h->port) + goto no_memory; + + VIR_FREE(disk->src); + disk->nhosts = 1; + disk->hosts = h; + return 0; + +no_memory: + virReportOOMError(); +error: + virDomainDiskHostDefFree(h); + VIR_FREE(h); + return -1; +} + +static int qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) { int ret = -1; @@ -2188,6 +2227,36 @@ no_memory: goto cleanup; } +static int +qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ + const char *transp; + + if (disk->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("nbd accepts only one host")); + return -1; + } + + virBufferAddLit(opt, "file=nbd:"); + + switch (disk->hosts->transport) { + case VIR_DOMAIN_DISK_PROTO_TRANS_TCP: + if (disk->hosts->name) + virBufferEscape(opt, ',', ",", "%s", disk->hosts->name); + virBufferEscape(opt, ',', ",", ":%s", + disk->hosts->port ? disk->hosts->port : "10809"); + break; + default: + transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("nbd does not support transport '%s'"), transp); + break; + } + + return 0; +} + char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, @@ -2314,13 +2383,9 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: - if (disk->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("NBD accepts only one host")); + if (qemuBuildNBDString(disk, &opt) < 0) goto error; - } - virBufferAsprintf(&opt, "file=nbd:%s:%s,", - disk->hosts->name, disk->hosts->port); + virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: virBufferAddLit(&opt, "file="); @@ -7337,39 +7402,11 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps, if (STRPREFIX(def->src, "/dev/")) def->type = VIR_DOMAIN_DISK_TYPE_BLOCK; else if (STRPREFIX(def->src, "nbd:")) { - char *host, *port; - def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; def->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; - host = def->src + strlen("nbd:"); - port = strchr(host, ':'); - if (!port) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse nbd filename '%s'"), - def->src); - goto error; - } - *port++ = '\0'; - if (VIR_ALLOC(def->hosts) < 0) { - virReportOOMError(); - goto error; - } - def->nhosts = 1; - def->hosts->name = strdup(host); - if (!def->hosts->name) { - virReportOOMError(); - goto error; - } - def->hosts->port = strdup(port); - if (!def->hosts->port) { - virReportOOMError(); - goto error; - } - def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; - def->hosts->socket = NULL; - VIR_FREE(def->src); - def->src = NULL; + if (qemuParseNBDString(def) < 0) + goto error; } else if (STRPREFIX(def->src, "rbd:")) { char *p = def->src; @@ -8599,7 +8636,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, else if (STRPREFIX(val, "nbd:")) { disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; - val += strlen("nbd:"); } else if (STRPREFIX(val, "rbd:")) { disk->type = VIR_DOMAIN_DISK_TYPE_NETWORK; disk->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; @@ -8639,26 +8675,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto no_memory; if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { - char *host, *port; + char *port; switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: - host = disk->src; - port = strchr(host, ':'); - if (!port) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse nbd filename '%s'"), disk->src); + if (qemuParseNBDString(disk) < 0) goto error; - } - *port++ = '\0'; - if (VIR_ALLOC(disk->hosts) < 0) - goto no_memory; - disk->nhosts = 1; - disk->hosts->name = host; - disk->hosts->port = strdup(port); - if (!disk->hosts->port) - goto no_memory; - disk->src = NULL; break; case VIR_DOMAIN_DISK_PROTOCOL_RBD: /* old-style CEPH_ARGS env variable is parsed later */ diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3f36896..2fa93a9 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -166,6 +166,7 @@ mymain(void) DO_TEST("disk-drive-cache-v1-wt"); DO_TEST("disk-drive-cache-v1-wb"); DO_TEST("disk-drive-cache-v1-none"); + DO_TEST("disk-drive-network-nbd"); DO_TEST("disk-scsi-device"); DO_TEST("disk-scsi-vscsi"); DO_TEST("disk-scsi-virtio-scsi"); -- 1.8.1.2

Il 11/03/2013 04:38, Osier Yang ha scritto: Ok. These are both zero in memory, and VIR_ALLOC guarantees that. Your choice. Paolo

On Mon, Feb 25, 2013 at 06:44:22PM +0100, Paolo Bonzini wrote: I would have had the 'parse' method further down near the other parse function which calls it, but no big deal. This test case addition could be pushed indepedently of the rest of this patch. ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 2013年02月26日 01:44, Paolo Bonzini wrote: if (!(src = strchr(port, ':'))) Likewise. ACK with the coding style changed to be consistent.

This reuses the XML format that was introduced for Gluster. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- docs/formatdomain.html.in | 8 ++-- src/qemu/qemu_command.c | 49 +++++++++++++++------- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-nbd-unix.args | 5 +++ .../qemuxml2argv-disk-drive-network-nbd-unix.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 80 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 90cfc03..094b509 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1713,9 +1713,11 @@ <td> only one </td> </tr> </table> - In case of gluster, valid values for transport attribute are tcp, rdma - or unix. If nothing is specified, tcp is assumed. If transport is unix, - socket attribute specifies path to unix socket. + gluster supports "tcp", "rdma", "unix" as valid values for the + transport attribute. nbd supports "tcp" and "unix". Others only + support "tcp". If nothing is specified, "tcp" is assumed. If the + transport is "unix", the socket attribute specifies the path to an + AF_UNIX socket. </dd> <dt><code>address</code></dt> <dd>If present, the <code>address</code> element ties the disk diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4762f0a..89cd065 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2138,25 +2138,34 @@ qemuParseNBDString(virDomainDiskDefPtr disk) goto no_memory; host = disk->src + strlen("nbd:"); - port = strchr(host, ':'); - if (!port) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse nbd filename '%s'"), disk->src); - goto error; - } + if (STRPREFIX(host, "unix:/")) { + src = strchr(host + strlen("unix:"), ':'); + if (src) + *src++ = '\0'; - *port++ = '\0'; - h->name = strdup(host); - if (!h->name) - goto no_memory; + h->transport = VIR_DOMAIN_DISK_PROTO_TRANS_UNIX; + h->socket = strdup(host + strlen("unix:")); + } else { + port = strchr(host, ':'); + if (!port) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse nbd filename '%s'"), disk->src); + goto error; + } - src = strchr(port, ':'); - if (src) - *src++ = '\0'; + *port++ = '\0'; + h->name = strdup(host); + if (!h->name) + goto no_memory; - h->port = strdup(port); - if (!h->port) - goto no_memory; + src = strchr(port, ':'); + if (src) + *src++ = '\0'; + + h->port = strdup(port); + if (!h->port) + goto no_memory; + } if (src && STRPREFIX(src, "exportname=")) { src = strdup(strchr(src, '=') + 1); @@ -2261,6 +2270,14 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) virBufferEscape(opt, ',', ",", ":%s", disk->hosts->port ? disk->hosts->port : "10809"); break; + case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX: + if (!disk->hosts->socket) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("socket attribute required for unix transport")); + return -1; + } + virBufferEscape(opt, ',', ",", "unix:%s", disk->hosts->socket); + break; default: transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 1731022..12174b3 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -183,6 +183,7 @@ mymain(void) DO_TEST("disk-drive-cache-unsafe"); DO_TEST("disk-drive-network-nbd"); DO_TEST("disk-drive-network-nbd-export"); + DO_TEST("disk-drive-network-nbd-unix"); DO_TEST("disk-drive-network-gluster"); DO_TEST("disk-drive-network-rbd"); DO_TEST("disk-drive-network-rbd-ipv6"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args new file mode 100644 index 0000000..afea187 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +file=nbd:unix:/var/run/nbdsock:exportname=bar,if=virtio,format=raw -net none -serial none \ +-parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml new file mode 100644 index 0000000..46114d5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml @@ -0,0 +1,33 @@ +<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/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='nbd' name='bar'> + <host transport='unix' socket='/var/run/nbdsock'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7d44be2..db09633 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -487,6 +487,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-nbd-export", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-drive-network-nbd-unix", + QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-gluster", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-rbd", diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index cdf6554..d698f59 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -168,6 +168,7 @@ mymain(void) DO_TEST("disk-drive-cache-v1-none"); DO_TEST("disk-drive-network-nbd"); DO_TEST("disk-drive-network-nbd-export"); + DO_TEST("disk-drive-network-nbd-unix"); DO_TEST("disk-scsi-device"); DO_TEST("disk-scsi-vscsi"); DO_TEST("disk-scsi-virtio-scsi"); -- 1.8.1.2

Il 11/03/2013 10:34, Osier Yang ha scritto: I think in this case the value that is assigned to src is complex enough that it's better to keep it separate. Yeah, we cannot be sure in general that it will be required for _all_ protocols, so I kept it here and followed what gluster does. Paolo

QEMU 1.3 and newer support an alternative URI-based syntax to specify the location of an NBD server. Libvirt can keep on using the old syntax in general, but only the URI syntax supports IPv6 addresses. The URI syntax also supports relative paths to Unix sockets. These should never be used but aren't explicitly blocked either by the parser, so support it just in case. The URI syntax is intentionally compatible with Gluster's, and the code can be reused. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- src/qemu/qemu_command.c | 97 +++++++++++++++------- tests/qemuargv2xmltest.c | 2 + ...ml2argv-disk-drive-network-nbd-ipv6-export.args | 5 ++ ...xml2argv-disk-drive-network-nbd-ipv6-export.xml | 33 ++++++++ .../qemuxml2argv-disk-drive-network-nbd-ipv6.args | 5 ++ .../qemuxml2argv-disk-drive-network-nbd-ipv6.xml | 33 ++++++++ tests/qemuxml2argvtest.c | 4 + tests/qemuxml2xmltest.c | 2 + 8 files changed, 153 insertions(+), 28 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89cd065..733d3bf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2050,35 +2050,36 @@ no_memory: } static int -qemuParseGlusterString(virDomainDiskDefPtr def) +qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, + const char *scheme) { int ret = -1; char *transp = NULL; char *sock = NULL; char *volimg = NULL; - virURIPtr uri = NULL; - - if (!(uri = virURIParse(def->src))) { - return -1; - } if (VIR_ALLOC(def->hosts) < 0) goto no_memory; - if (STREQ(uri->scheme, "gluster")) { + transp = strchr(uri->scheme, '+'); + if (transp) + *transp++ = 0; + + if (!STREQ(uri->scheme, scheme)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid transport/scheme '%s'"), uri->scheme); + goto error; + } + + if (!transp) { def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; - } else if (STRPREFIX(uri->scheme, "gluster+")) { - transp = strchr(uri->scheme, '+') + 1; + } else { def->hosts->transport = virDomainDiskProtocolTransportTypeFromString(transp); if (def->hosts->transport < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid gluster transport type '%s'"), transp); + _("Invalid %s transport type '%s'"), scheme, transp); goto error; } - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid transport/scheme '%s'"), uri->scheme); - goto error; } def->nhosts = 0; /* set to 1 once everything succeeds */ @@ -2105,11 +2106,16 @@ qemuParseGlusterString(virDomainDiskDefPtr def) } } } - volimg = uri->path + 1; /* skip the prefix slash */ - VIR_FREE(def->src); - def->src = strdup(volimg); - if (!def->src) - goto no_memory; + if (uri->path) { + volimg = uri->path + 1; /* skip the prefix slash */ + VIR_FREE(def->src); + def->src = strdup(volimg); + if (!def->src) + goto no_memory; + } else { + VIR_FREE(def->src); + def->src = NULL; + } def->nhosts = 1; ret = 0; @@ -2128,12 +2134,31 @@ error: } static int +qemuParseGlusterString(virDomainDiskDefPtr def) +{ + virURIPtr uri = NULL; + + if (!(uri = virURIParse(def->src))) + return -1; + + return qemuParseDriveURIString(def, uri, "gluster"); +} + +static int qemuParseNBDString(virDomainDiskDefPtr disk) { virDomainDiskHostDefPtr h = NULL; char *host, *port; char *src; + virURIPtr uri = NULL; + + if (strstr(disk->src, "://")) { + uri = virURIParse(disk->src); + if (uri) + return qemuParseDriveURIString(disk, uri, "nbd"); + } + if (VIR_ALLOC(h) < 0) goto no_memory; @@ -2190,7 +2215,8 @@ error: } static int -qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt, + const char *scheme) { int ret = -1; int port = 0; @@ -2204,18 +2230,18 @@ qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) }; if (disk->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("gluster accepts only one host")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s accepts only one host"), scheme); return -1; } virBufferAddLit(opt, "file="); transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); - if (virAsprintf(&tmpscheme, "gluster+%s", transp) < 0) + if (virAsprintf(&tmpscheme, "%s+%s", scheme, transp) < 0) goto no_memory; - if (virAsprintf(&volimg, "/%s", disk->src) < 0) + if (disk->src && virAsprintf(&volimg, "/%s", disk->src) < 0) goto no_memory; if (disk->hosts->port) { @@ -2251,16 +2277,29 @@ no_memory: } static int +qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +{ + return qemuBuildDriveURIString(disk, opt, "gluster"); +} + +static int qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) { const char *transp; if (disk->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("nbd accepts only one host")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s accepts only one host"), "nbd"); return -1; } + if ((disk->hosts->name && strchr(disk->hosts->name, ':')) + || (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP + && !disk->hosts->name) + || (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX + && disk->hosts->socket && disk->hosts->socket[0] != '/')) + return qemuBuildDriveURIString(disk, opt, "nbd"); + virBufferAddLit(opt, "file=nbd:"); switch (disk->hosts->transport) { @@ -7435,7 +7474,8 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps, values[i] = NULL; if (STRPREFIX(def->src, "/dev/")) def->type = VIR_DOMAIN_DISK_TYPE_BLOCK; - else if (STRPREFIX(def->src, "nbd:")) { + else if (STRPREFIX(def->src, "nbd:") || + STRPREFIX(def->src, "nbd+")) { def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; def->protocol = VIR_DOMAIN_DISK_PROTOCOL_NBD; @@ -7456,7 +7496,8 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps, goto cleanup; VIR_FREE(p); - } else if (STRPREFIX(def->src, "gluster")) { + } else if (STRPREFIX(def->src, "gluster:") || + STRPREFIX(def->src, "gluster+")) { def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; def->protocol = VIR_DOMAIN_DISK_PROTOCOL_GLUSTER; diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 12174b3..9fbba94 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -183,6 +183,8 @@ mymain(void) DO_TEST("disk-drive-cache-unsafe"); DO_TEST("disk-drive-network-nbd"); DO_TEST("disk-drive-network-nbd-export"); + DO_TEST("disk-drive-network-nbd-ipv6"); + DO_TEST("disk-drive-network-nbd-ipv6-export"); DO_TEST("disk-drive-network-nbd-unix"); DO_TEST("disk-drive-network-gluster"); DO_TEST("disk-drive-network-rbd"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args new file mode 100644 index 0000000..e12d4d6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +'file=nbd+tcp://[::1]:6000/bar,if=virtio,format=raw' -net none -serial none \ +-parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml new file mode 100644 index 0000000..595d7ea --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml @@ -0,0 +1,33 @@ +<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/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='nbd' name='bar'> + <host name='::1' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args new file mode 100644 index 0000000..e814805 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +'file=nbd+tcp://[::1]:6000,if=virtio,format=raw' -net none -serial none \ +-parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml new file mode 100644 index 0000000..3c5c99d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml @@ -0,0 +1,33 @@ +<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/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='nbd'> + <host name='::1' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index db09633..35cba01 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -487,6 +487,10 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-nbd-export", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-drive-network-nbd-ipv6", + QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-drive-network-nbd-ipv6-export", + QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-nbd-unix", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-gluster", diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d698f59..7a01954 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -168,6 +168,8 @@ mymain(void) DO_TEST("disk-drive-cache-v1-none"); DO_TEST("disk-drive-network-nbd"); DO_TEST("disk-drive-network-nbd-export"); + DO_TEST("disk-drive-network-nbd-ipv6"); + DO_TEST("disk-drive-network-nbd-ipv6-export"); DO_TEST("disk-drive-network-nbd-unix"); DO_TEST("disk-scsi-device"); DO_TEST("disk-scsi-vscsi"); -- 1.8.1.2

On 2013年02月26日 01:44, Paolo Bonzini wrote: Worth to add "<span class="since">since 1.0.4</span>"? ACK with the question kept.

On 2013年02月26日 01:44, Paolo Bonzini wrote: Why not keeping the "+tcp" for Gluster and NBD instead? I'm afraid of of relying on qemu's default. Can it be changed? Osier

On 2013年03月11日 21:22, Paolo Bonzini wrote: Okay, I believe you, ACK.

Only sheepdog actually required it in the code, and we can use 7000 as the default---the same value that QEMU uses for the simple "sheepdog:VOLUME" syntax. With this change, the schema can be fixed to allow no port. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 8 +++++--- src/conf/domain_conf.c | 5 ----- src/qemu/qemu_command.c | 3 ++- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5b4eabe..c590427 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1692,31 +1692,37 @@ <th> Protocol </th> <th> Meaning </th> <th> Number of hosts </th> + <th> Default port </th> </tr> <tr> <td> nbd </td> <td> a server running nbd-server </td> <td> only one </td> + <td> 10809 </td> </tr> <tr> <td> iscsi </td> <td> an iSCSI server </td> <td> only one </td> + <td> 3260 </td> </tr> <tr> <td> rbd </td> <td> monitor servers of RBD </td> <td> one or more </td> + <td> 6789 </td> </tr> <tr> <td> sheepdog </td> <td> one of the sheepdog servers (default is localhost:7000) </td> <td> zero or one </td> + <td> 7000 </td> </tr> <tr> <td> gluster </td> <td> a server running glusterd daemon </td> <td> only one </td> + <td> 24007 </td> </tr> </table> gluster supports "tcp", "rdma", "unix" as valid values for the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 82062a3..b8c4503 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1119,9 +1119,11 @@ <ref name="ipAddr"/> </choice> </attribute> - <attribute name="port"> - <ref name="unsignedInt"/> - </attribute> + <optional> + <attribute name="port"> + <ref name="unsignedInt"/> + </attribute> + </optional> </group> <group> <attribute name="transport"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b801239..71da694 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4013,11 +4013,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, goto error; } hosts[nhosts - 1].port = virXMLPropString(child, "port"); - if (!hosts[nhosts - 1].port) { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("missing port for host")); - goto error; - } } } child = child->next; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 59773ec..5d52be5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2526,7 +2526,8 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } else { /* only one host is supported now */ virBufferAsprintf(&opt, "file=sheepdog:%s:%s:", - disk->hosts->name, disk->hosts->port); + disk->hosts->name, + disk->hosts->port ? disk->hosts->port : "7000"); virBufferEscape(&opt, ',', ",", "%s,", disk->src); } break; -- 1.8.1.2

Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- docs/formatsecret.html.in | 12 ++++++++++++ docs/schemas/secret.rng | 10 ++++++++++ include/libvirt/libvirt.h.in | 1 + src/conf/secret_conf.c | 22 +++++++++++++++++++++- src/conf/secret_conf.h | 1 + src/secret/secret_driver.c | 8 ++++++++ 6 files changed, 53 insertions(+), 1 deletion(-) diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 01aff2d..c3c4a25 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -66,6 +66,18 @@ device</a>. <span class="since">Since 0.9.7</span>. </p> + <h3>Usage type "iscsi"</h3> + + <p> + This secret is associated with an iSCSI target for CHAP authentication. + The <code>&lt;usage type='iscsi'&gt;</code> element must contain + a single <code>target</code> element that specifies a usage name + for the secret. The iSCSI secret can then be used by UUID or by + this usage name via the <code>&lt;auth&gt;</code> element of + a <a href="domain.html#elementsDisks">disk + device</a>. <span class="since">Since 1.0.4</span>. + </p> + <h2><a name="example">Example</a></h2> <pre> diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index e49bd5a..d7b8f83 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -41,6 +41,7 @@ <choice> <ref name='usagevolume'/> <ref name='usageceph'/> + <ref name='usageiscsi'/> <!-- More choices later --> </choice> </element> @@ -67,4 +68,13 @@ </element> </define> + <define name='usageiscsi'> + <attribute name='type'> + <value>iscsi</value> + </attribute> + <element name='target'> + <ref name='genericName'/> + </element> + </define> + </grammar> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ad30cd0..3f98ebe 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3641,6 +3641,7 @@ typedef enum { VIR_SECRET_USAGE_TYPE_NONE = 0, VIR_SECRET_USAGE_TYPE_VOLUME = 1, VIR_SECRET_USAGE_TYPE_CEPH = 2, + VIR_SECRET_USAGE_TYPE_ISCSI = 3, #ifdef VIR_ENUM_SENTINELS VIR_SECRET_USAGE_TYPE_LAST diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 891af65..06b9bb2 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -36,7 +36,7 @@ #define VIR_FROM_THIS VIR_FROM_SECRET VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST, - "none", "volume", "ceph") + "none", "volume", "ceph", "iscsi") void virSecretDefFree(virSecretDefPtr def) @@ -57,6 +57,10 @@ virSecretDefFree(virSecretDefPtr def) VIR_FREE(def->usage.ceph); break; + case VIR_SECRET_USAGE_TYPE_ISCSI: + VIR_FREE(def->usage.target); + break; + default: VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type); break; @@ -108,6 +112,15 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, } break; + case VIR_SECRET_USAGE_TYPE_ISCSI: + def->usage.target = virXPathString("string(./usage/target)", ctxt); + if (!def->usage.target) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Ceph usage specified, but target is missing")); + return -1; + } + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), @@ -262,6 +275,13 @@ virSecretDefFormatUsage(virBufferPtr buf, } break; + case VIR_SECRET_USAGE_TYPE_ISCSI: + if (def->usage.target != NULL) { + virBufferEscapeString(buf, " <target>%s</target>\n", + def->usage.target); + } + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index 6079d5b..53517f9 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -39,6 +39,7 @@ struct _virSecretDef { union { char *volume; /* May be NULL */ char *ceph; + char *target; } usage; }; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 5be33b9..c577817 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -149,6 +149,11 @@ secretFindByUsage(virSecretDriverStatePtr driver, int usageType, const char *usa if (STREQ(s->def->usage.ceph, usageID)) return s; break; + + case VIR_SECRET_USAGE_TYPE_ISCSI: + if (STREQ(s->def->usage.target, usageID)) + return s; + break; } } return NULL; @@ -614,6 +619,9 @@ secretUsageIDForDef(virSecretDefPtr def) case VIR_SECRET_USAGE_TYPE_CEPH: return def->usage.ceph; + case VIR_SECRET_USAGE_TYPE_ISCSI: + return def->usage.target; + default: return NULL; } -- 1.8.1.2

Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- docs/formatdomain.html.in | 12 ++++----- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 31 ++++++++++++++++------ .../qemuxml2argv-disk-drive-network-iscsi-auth.xml | 31 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c590427..0906fe9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1760,12 +1760,12 @@ holds the actual password or other credentials (the domain XML intentionally does not expose the password, only the reference to the object that does manage the password). For now, the - only known secret <code>type</code> is "ceph", for Ceph RBD - network sources, and requires either an - attribute <code>uuid</code> with the UUID of the Ceph secret - object, or an attribute <code>usage</code> with the name - associated with the Ceph secret - object. <span class="since">libvirt 0.9.7</span> + known secret <code>type</code>s are "ceph", for Ceph RBD + network sources, and "iscsi", for CHAP authentication of iSCSI + targets. Both require either a <code>uuid</code> attribute + with the UUID of the secret object, or a <code>usage</code> + attribute matching the key that was specified in the + secret object. <span class="since">libvirt 0.9.7</span> </dd> <dt><code>geometry</code></dt> <dd>The optional <code>geometry</code> element provides the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b8c4503..6f85e84 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3592,6 +3592,7 @@ <attribute name='type'> <choice> <value>ceph</value> + <value>iscsi</value> </choice> </attribute> <choice> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 71da694..e4c3e67 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3885,6 +3885,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *wwn = NULL; char *vendor = NULL; char *product = NULL; + int expected_secret_usage = -1; + int auth_secret_usage = -1; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -3922,7 +3924,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (cur->type == XML_ELEMENT_NODE) { if (!source && !hosts && xmlStrEqual(cur->name, BAD_CAST "source")) { - sourceNode = cur; switch (def->type) { @@ -3958,6 +3959,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, _("invalid logical unit number")); goto error; } + expected_secret_usage = VIR_SECRET_USAGE_TYPE_ISCSI; + } else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + expected_secret_usage = VIR_SECRET_USAGE_TYPE_CEPH; } if (!(source = virXMLPropString(cur, "name")) && def->protocol != VIR_DOMAIN_DISK_PROTOCOL_NBD) { @@ -4144,8 +4148,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, _("missing type for secret")); goto error; } - if (virSecretUsageTypeTypeFromString(usageType) != - VIR_SECRET_USAGE_TYPE_CEPH) { + auth_secret_usage = + virSecretUsageTypeTypeFromString(usageType); + if (auth_secret_usage < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid secret type %s"), usageType); @@ -4295,6 +4300,13 @@ virDomainDiskDefParseXML(virCapsPtr caps, cur = cur->next; } + if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid secret type '%s'"), + virSecretUsageTypeTypeToString(auth_secret_usage)); + goto error; + } + device = virXMLPropString(node, "device"); if (device) { if ((def->device = virDomainDiskDeviceTypeFromString(device)) < 0) { @@ -12500,15 +12512,18 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->auth.username) { virBufferEscapeString(buf, " <auth username='%s'>\n", def->auth.username); + if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { + virBufferAsprintf(buf, " <secret type='iscsi'"); + } else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { + virBufferAsprintf(buf, " <secret type='ceph'"); + } + if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_UUID) { virUUIDFormat(def->auth.secret.uuid, uuidstr); - virBufferAsprintf(buf, - " <secret type='ceph' uuid='%s'/>\n", - uuidstr); + virBufferAsprintf(buf, " uuid='%s'/>\n", uuidstr); } if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) { - virBufferEscapeString(buf, - " <secret type='ceph' usage='%s'/>\n", + virBufferEscapeString(buf, " usage='%s'/>\n", def->auth.secret.usage); } virBufferAddLit(buf, " </auth>\n"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml new file mode 100644 index 0000000..acaa503 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml @@ -0,0 +1,31 @@ +<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/bin/qemu</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e0d3b20..076bf79 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -172,6 +172,7 @@ mymain(void) DO_TEST("disk-drive-network-nbd-ipv6-export"); DO_TEST("disk-drive-network-nbd-unix"); DO_TEST("disk-drive-network-iscsi"); + DO_TEST("disk-drive-network-iscsi-auth"); DO_TEST("disk-scsi-device"); DO_TEST("disk-scsi-vscsi"); DO_TEST("disk-scsi-virtio-scsi"); -- 1.8.1.2

A better way to do this would be to use a configuration file like [iscsi "target-name"] user = name password = pwd and pass it via -readconfig. This would remove the username and password from the "ps" output. For now, however, keep this solution. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- src/qemu/qemu_command.c | 80 ++++++++++++++++++---- ...qemuxml2argv-disk-drive-network-iscsi-auth.args | 1 + tests/qemuxml2argvtest.c | 2 + 3 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d52be5..30ddbd3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1888,8 +1888,8 @@ qemuBuildRBDString(virConnectPtr conn, VIR_FREE(base64); } else { virReportError(VIR_ERR_INTERNAL_ERROR, - _("rbd username '%s' specified but secret not found"), - disk->auth.username); + _("%s username '%s' specified but secret not found"), + "rbd", disk->auth.username); goto error; } } else { @@ -2057,6 +2057,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, char *transp = NULL; char *sock = NULL; char *volimg = NULL; + char *secret = NULL; if (VIR_ALLOC(def->hosts) < 0) goto no_memory; @@ -2117,6 +2118,16 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, def->src = NULL; } + if (uri->user) { + secret = strchr(uri->user, ':'); + if (secret) + *secret = '\0'; + + def->auth.username = strdup(uri->user); + if (!def->auth.username) + goto no_memory; + } + def->nhosts = 1; ret = 0; @@ -2237,14 +2248,20 @@ error: } static int -qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt, - const char *scheme) +qemuBuildDriveURIString(virConnectPtr conn, + virDomainDiskDefPtr disk, virBufferPtr opt, + const char *scheme, virSecretUsageType secretType) { int ret = -1; int port = 0; + virSecretPtr sec = NULL; + char *secret = NULL; + size_t secret_size; + char *tmpscheme = NULL; char *volimg = NULL; char *sock = NULL; + char *user = NULL; char *builturi = NULL; const char *transp = NULL; virURI uri = { @@ -2280,8 +2297,42 @@ qemuBuildDriveURIString(virDomainDiskDefPtr disk, virBufferPtr opt, virAsprintf(&sock, "socket=%s", disk->hosts->socket) < 0) goto no_memory; + if (disk->auth.username && secretType != VIR_SECRET_USAGE_TYPE_NONE) { + /* look up secret */ + switch (disk->auth.secretType) { + case VIR_DOMAIN_DISK_SECRET_TYPE_UUID: + sec = virSecretLookupByUUID(conn, + disk->auth.secret.uuid); + break; + case VIR_DOMAIN_DISK_SECRET_TYPE_USAGE: + sec = virSecretLookupByUsage(conn, secretType, + disk->auth.secret.usage); + break; + } + + if (sec) { + secret = (char *)conn->secretDriver->getValue(sec, &secret_size, 0, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); + if (secret == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("could not get the value of the secret for username %s"), + disk->auth.username); + ret = -1; + goto cleanup; + } + if (virAsprintf(&user, "%s:%s", disk->auth.username, secret) < 0) + goto no_memory; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s username '%s' specified but secret not found"), + scheme, disk->auth.username); + ret = -1; + goto cleanup; + } + } uri.scheme = tmpscheme; /* gluster+<transport> */ uri.server = disk->hosts->name; + uri.user = user; uri.port = port; uri.path = volimg; uri.query = sock; @@ -2305,16 +2356,18 @@ no_memory: } static int -qemuBuildGlusterString(virDomainDiskDefPtr disk, virBufferPtr opt) +qemuBuildGlusterString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) { - return qemuBuildDriveURIString(disk, opt, "gluster"); + return qemuBuildDriveURIString(conn, disk, opt, "gluster", + VIR_SECRET_USAGE_TYPE_NONE); } static int -qemuBuildISCSIString(virDomainDiskDefPtr disk, virBufferPtr opt) +qemuBuildISCSIString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) { int ret; - ret = qemuBuildDriveURIString(disk, opt, "iscsi"); + ret = qemuBuildDriveURIString(conn, disk, opt, "iscsi", + VIR_SECRET_USAGE_TYPE_ISCSI); if (ret < 0) return ret; @@ -2325,7 +2378,7 @@ qemuBuildISCSIString(virDomainDiskDefPtr disk, virBufferPtr opt) } static int -qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) +qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr opt) { const char *transp; @@ -2340,7 +2393,8 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt) && !disk->hosts->name) || (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_UNIX && disk->hosts->socket && disk->hosts->socket[0] != '/')) - return qemuBuildDriveURIString(disk, opt, "nbd"); + return qemuBuildDriveURIString(conn, disk, opt, "nbd", + VIR_SECRET_USAGE_TYPE_NONE); virBufferAddLit(opt, "file=nbd:"); @@ -2498,7 +2552,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: - if (qemuBuildNBDString(disk, &opt) < 0) + if (qemuBuildNBDString(conn, disk, &opt) < 0) goto error; virBufferAddChar(&opt, ','); break; @@ -2509,12 +2563,12 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: - if (qemuBuildGlusterString(disk, &opt) < 0) + if (qemuBuildGlusterString(conn, disk, &opt) < 0) goto error; virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_ISCSI: - if (qemuBuildISCSIString(disk, &opt) < 0) + if (qemuBuildISCSIString(conn, disk, &opt) < 0) goto error; virBufferAddChar(&opt, ','); break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args new file mode 100644 index 0000000..fd2660a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive file=iscsi://myname:AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A@example.org/iqn.1992-01.com.example,if=virtio,format=raw -net none -serial none -parallel none diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index babdd8c..0afecf3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -495,6 +495,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-iscsi", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-drive-network-iscsi-auth", + QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-gluster", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-rbd", -- 1.8.1.2

This enables usage of commands like persistent reservations. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 9 ++++++- .../qemuxml2argv-disk-drive-network-iscsi-lun.args | 1 + .../qemuxml2argv-disk-drive-network-iscsi-lun.xml | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 5 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f399871..8acff1a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -156,6 +156,7 @@ virDomainDiskIoTypeToString; virDomainDiskPathByName; virDomainDiskProtocolTransportTypeFromString; virDomainDiskProtocolTransportTypeToString; +virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; virDomainDiskTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 30ddbd3..22e9d81 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2844,7 +2844,14 @@ qemuBuildDriveDevStr(virDomainDefPtr def, bus); goto error; } - if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK) { + if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + if (disk->protocol != VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk device='lun' is not supported for protocol='%s'"), + virDomainDiskProtocolTypeToString(disk->protocol)); + goto error; + } + } else if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk device='lun' is not supported for type='%s'"), virDomainDiskTypeToString(disk->type)); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args new file mode 100644 index 0000000..baa7760 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb -drive file=iscsi://example.org:3260/iqn.1992-01.com.example,if=none,id=drive-scsi0-0-0-0,format=raw -device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml new file mode 100644 index 0000000..b6d6aaa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.xml @@ -0,0 +1,28 @@ +<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/bin/qemu</emulator> + <disk type='network' device='lun'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='3260'/> + </source> + <target dev='sda' bus='scsi'/> + </disk> + <controller type='usb' index='0'/> + <controller type='scsi' model='virtio-scsi'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0afecf3..5e0d976 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -497,6 +497,10 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-iscsi-auth", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-drive-network-iscsi-lun", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE_FORMAT, + QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI_PCI, + QEMU_CAPS_VIRTIO_BLK_SG_IO, QEMU_CAPS_SCSI_BLOCK); DO_TEST("disk-drive-network-gluster", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-rbd", -- 1.8.1.2