Sample XML for a VxHS disk: <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(a)veritas.com&gt; --- v5 changelog: (1) Rebased to latest master. (2) Review comments from Peter Krempa on patch v4 1/3 have been addressed v4 changelog: (1) Fixes per review comments from v3. (2) Had to remove a test from the previous version that checked for error when multiple hosts are specified for VxHS device. This started failing virschematest with the error "XML document failed to validate against schema" as the docs/schemas/domain.rng specifies only a single host. v3 changelog: (1) Implemented the modern syntax for VxHS disk specification. (2) Changed qemuxml2argvdata VxHS test case to verify the new syntax. (3) Added a negative test case to check failure when multiple hosts are specified for a VxHS disk. v2 changelog: (1) Added code for JSON parsing of a VxHS vdisk. (2) Added test case to verify JSON parsing. (3) Added missing switch-case checks for VIR_STORAGE_NET_PROTOCOL_VXHS. (4) Fixed line wrap in qemuxml2argv-disk-drive-network-vxhs.args. docs/formatdomain.html.in | 15 ++++++++--- docs/schemas/domaincommon.rng | 13 ++++++++++ src/libxl/libxl_conf.c | 1 + src/qemu/qemu_block.c | 60 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.c | 9 +++++++ src/qemu/qemu_driver.c | 3 +++ src/qemu/qemu_parse_command.c | 15 +++++++++++ src/util/virstoragefile.c | 40 ++++++++++++++++++++++++++++- src/util/virstoragefile.h | 1 + src/xenconfig/xen_xl.c | 1 + 10 files changed, 154 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fba8cfc..64397d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2520,9 +2520,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> @@ -2530,6 +2530,9 @@ target's name by a slash (e.g., <code>iqn.2013-07.com.example:iscsi-pool/1</code>). If not specified, the default LUN is zero. + For "vxhs" (<span class="since">since 3.8.0</span>), the + <code>name</code> is the UUID of the volume, assigned by the + HyperScale sever. <span class="since">Since 0.8.7</span> </dd> <dt><code>volume</code></dt> @@ -2632,6 +2635,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 3f56d8f..458b8d8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1642,6 +1642,18 @@ </element> </define> + <define name="diskSourceNetworkProtocolVxHS"> + <element name="source"> + <attribute name="protocol"> + <choice> + <value>vxhs</value> + </choice> + </attribute> + <attribute name="name"/> + <ref name="diskSourceNetworkHost"/> + </element> + </define> + <define name="diskSourceNetwork"> <attribute name="type"> <value>network</value> @@ -1652,6 +1664,7 @@ <ref name="diskSourceNetworkProtocolRBD"/> <ref name="diskSourceNetworkProtocolHTTP"/> <ref name="diskSourceNetworkProtocolSimple"/> + <ref name="diskSourceNetworkProtocolVxHS"/> </choice> </define> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4416a09..34233a9 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -666,6 +666,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_GLUSTER: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_SSH: + case VIR_STORAGE_NET_PROTOCOL_VXHS: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7fb12ea..a4d0160 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -23,6 +23,7 @@ #include "viralloc.h" #include "virstring.h" +#include "qemu_alias.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -482,6 +483,60 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBuildVxHSDriveJSONHost(virStorageSourcePtr src) +{ + virJSONValuePtr server = NULL; + virStorageNetHostDefPtr host; + unsigned int port; + + if (src->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("protocol VxHS accepts only one host")); + return NULL; + } + + host = src->hosts; + port = host->port; + + if (virJSONValueObjectCreate(&server, + "s:host", host->name, + "u:port", port, + NULL) < 0) + server = NULL; + + return server; +} + + +static virJSONValuePtr +qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src) +{ + const char *protocol = virStorageNetProtocolTypeToString(src->protocol); + virJSONValuePtr server = NULL; + virJSONValuePtr ret = NULL; + + if (!(server = qemuBuildVxHSDriveJSONHost(src))) + return NULL; + + /* VxHS disk specification example: + * { driver:"vxhs", + * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251", + * server.host:"1.2.3.4", + * server.port:1234} + */ + if (virJSONValueObjectCreate(&ret, + "s:driver", protocol, + "s:vdisk-id", src->path, + "a:server", server, NULL) < 0) { + virJSONValueFree(server); + ret = NULL; + } + + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -512,6 +567,11 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) goto cleanup; break; + case VIR_STORAGE_NET_PROTOCOL_VXHS: + if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src))) + goto cleanup; + break; + case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a68ff71..0fd2674 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -991,6 +991,11 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, ret = virBufferContentAndReset(&buf); break; + case VIR_STORAGE_NET_PROTOCOL_VXHS: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'VxHS' protocol does not support URI syntax")); + goto cleanup; + case VIR_STORAGE_NET_PROTOCOL_SSH: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'ssh' protocol is not yet supported")); @@ -1325,6 +1330,10 @@ qemuDiskSourceNeedsProps(virStorageSourcePtr src) src->nhosts > 1) return true; + if (actualType == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) + return true; + return false; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2ba6c80..da28c4f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13732,6 +13732,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 " @@ -13795,6 +13796,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 " @@ -13940,6 +13942,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 8cb96a2..6286c2e 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -736,6 +736,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (VIR_STRDUP(def->src->path, vdi) < 0) goto error; } + } else if (STRPREFIX(def->src->path, "vxhs:")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("VxHS protocol does not support URI syntax '%s'"), + def->src->path); + goto error; } else { def->src->type = VIR_STORAGE_TYPE_FILE; } @@ -1944,6 +1949,10 @@ qemuParseCommandLine(virCapsPtr caps, disk->src->type = VIR_STORAGE_TYPE_NETWORK; disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_SHEEPDOG; val += strlen("sheepdog:"); + } else if (STRPREFIX(val, "vxhs:")) { + disk->src->type = VIR_STORAGE_TYPE_NETWORK; + disk->src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS; + val += strlen("vxhs:"); } else { disk->src->type = VIR_STORAGE_TYPE_FILE; } @@ -2020,6 +2029,12 @@ qemuParseCommandLine(virCapsPtr caps, goto error; break; + case VIR_STORAGE_NET_PROTOCOL_VXHS: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("VxHS protocol does not support URI " + "syntax '%s'"), disk->src->path); + goto error; + break; case VIR_STORAGE_NET_PROTOCOL_HTTP: case VIR_STORAGE_NET_PROTOCOL_HTTPS: case VIR_STORAGE_NET_PROTOCOL_FTP: diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index fbc8245..e9a59e0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -85,7 +85,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", @@ -2712,6 +2713,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); @@ -3210,6 +3212,38 @@ virStorageSourceParseBackingJSONRaw(virStorageSourcePtr src, return virStorageSourceParseBackingJSONInternal(src, json); } +static int +virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src, + virJSONValuePtr json, + int opaque ATTRIBUTE_UNUSED) +{ + const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id"); + virJSONValuePtr server = virJSONValueObjectGetObject(json, "server"); + + if (!vdisk_id || !server) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("missing 'vdisk-id' or 'server' attribute in " + "JSON backing definition for VxHS volume")); + return -1; + } + + src->type = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_VXHS; + + if (VIR_STRDUP(src->path, vdisk_id) < 0) + return -1; + + if (VIR_ALLOC_N(src->hosts, 1) < 0) + return -1; + src->nhosts = 1; + + if (virStorageSourceParseBackingJSONInetSocketAddress(src->hosts, + server) < 0) + return -1; + + return 0; +} + struct virStorageSourceJSONDriverParser { const char *drvname; int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque); @@ -3232,6 +3266,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"ssh", virStorageSourceParseBackingJSONSSH, 0}, {"rbd", virStorageSourceParseBackingJSONRBD, 0}, {"raw", virStorageSourceParseBackingJSONRaw, 0}, + {"vxhs", virStorageSourceParseBackingJSONVxHS, 0}, }; @@ -3992,6 +4027,9 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol) /* we don't provide a default for RBD */ return 0; + case VIR_STORAGE_NET_PROTOCOL_VXHS: + return 9999; + case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: return 0; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 6c388b1..f7e897f 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -134,6 +134,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/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index d168d3f..8acbfe3 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -1024,6 +1024,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src) case VIR_STORAGE_NET_PROTOCOL_GLUSTER: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_SSH: + case VIR_STORAGE_NET_PROTOCOL_VXHS: case VIR_STORAGE_NET_PROTOCOL_LAST: case VIR_STORAGE_NET_PROTOCOL_NONE: virReportError(VIR_ERR_NO_SUPPORT, -- 2.5.5

Thanks for all the reviews! I will work on each item and get back. On Tue, Aug 29, 2017 at 4:00 PM, John Ferlan <jferlan(a)redhat.com&gt; wrote: Work email please. Thanks! Thanks! Appreciate the help! Regards, Ashish

On 08/29/2017 02:39 AM, Ashish Mittal wrote: Could have been merged with previous, but as long as there's tests that's good. I'll add a qemuxml2xmltest.c option and a qemuxml2xmlout-disk-drive-network-vxhs.xml file There would be an adjustment to the .args output due the "file.server.0" now instead of just "file.server" and of course the "file.server.0.type = tcp,". I assume that works with the qemu changes anyway. This is unnecessary, I'll remove (it'll cause xml2xml errors too) Likewise, so I'll remove John

On 08/29/2017 02:39 AM, Ashish Mittal wrote: Similarly this probably should be merged with the code that alters qemu_{command|block}... For consistency, I'll modify port to be 9999 and I'll add the "type:tcp," string as well to match previous change John

On 08/29/2017 02:39 AM, Ashish Mittal wrote: Starting with this patch - I think there's a bit more work to do... We just need to think logically through things based on the VxHS usage model. s/(QEMU only) // s/vxhs/VxHS/ I see this is pretty much a copy of the chardev TLS wording; however, I'm not quite sure the exact same logic can apply in what you've done in this patch. Those last 3 lines may not be necessary - I guess that depends on what happens with how you handle or describe the "default" environment. Is there a reason you've added them and the check in the next patch? BTW: If they stay, it'd be the "domain disk source specification" to be specific. I don't believe you can use/rely on default like the other definitions and that's something you're going to have to "call out" if you want to claim someone could use the default cert_dir. If the default environment didn't set _verify, then theoretically it won't have the client-cert.pem and client-key.pem files. What's not 100% clear is whether the VxHS server would require them. What is clear is that the QEMU block/vxhs.c code would provide them and that from what I've read iio_open() requires them. As for the default environment the _uuid value - that doesn't matter because it's used to decode the server.pem file and vxhs doesn't use that for the "client" side. This sets the default to /etc/pki/qemu if /etc/pki/libvirt-vxhs doesn't exist. The existence of /etc/pki/qemu is not checked until later... In the time since you've last made changes in this space and now there's been some other adjustments which you'll need to add vxhs specific code for. See virQEMUDriverConfigTLSDirResetDefaults and add a CHECK_RESET_CERT_DIR_DEFAULT(vxhs); See virQEMUDriverConfigValidate to make the appropriate changes. If you've decided that you're going to allow usage of the default environment, then the verify code would have to ensure that if vxhs_tls were set that default verify was true. The default uuid has no bearing. If those conditions aren't met, then you'd have to fail the config setup. John

On 08/29/2017 02:39 AM, Ashish Mittal wrote: TLS CA certificate and the client certificate and keys are saved. These three points I think belong in the previous patch commit message... (4) and (5) somewhat contradict each other, but we can work through this. Additionally, let's use cfg->vxhsTLS or disk->source->haveTLS What this patch does is use the config->vxhsTLS along with the introduced disk->src->haveTLS in order to add TLS data to the VxHS client connection. s/domain/domain disk source/ I'm still not clear why this is a requirement if in fact you can really use the default qemu environment. So this patch is where things are going to get tricky. On one hand, trying to create a generic means to add/manage a 'tls' attribute for a disk source needs to be considered; however, this patch implements it strictly for VxHS, which may not be the best thing. Thus, I think there needs to be a patch prior to this that adds a generic/optional "tls" attribute to diskSourceNetworkHost. No one will use it yet, but it'll be there. Part of that patch would modify/introduce qemuDomainPrepareDiskSource to be run after qemuDomainPrepareChardevSource and before qemuDomainSecretPrepare. It's sole purpose would be to run through the disk array looking for disks to set up I think rather than here in the VxHS specific code - perhaps we add tls to the diskSourceNetworkHost. I need to investigate that. If that's done, then a patch before this one would be created. Adjust the above to be a multi-line comment... e.g. /* words * more words */ It's just a consistency thing. flip-flop conditional - no need to check for it for other protocols (yet)... Besides that's a longer trip - let's make the quick check first. Adjust to remove "VxHS" since it could be generic some day... Replace with "disk source" And now we add in the qemu_alias.h include NB: The setting of ret = NULL on the error path was unnecessary Once you dig down into the depths of virJSONValueObjectCreate you'd find that virJSONValueObjectAddVArgs will allow passing a NULL argument by using a capital letter. If 'S:tls-creds' is used, then if tls-creds == NULL, then we don't print the string. So rather than the hunk above, we can modify the: if (virJSONValueObjectCreate(&ret, "s:driver", protocol, "s:vdisk-id", src->path, "a:server", server, NULL) < 0) { to be if (virJSONValueObjectCreate(&ret, "s:driver", protocol, "S:tls-creds", objalias, "s:vdisk-id", src->path, "a:server", server, NULL) < 0) { and only add: if (src->addTLS && !(objalias = qemuAliasTLSObjFromSrcAlias("vxhs"))) { virJSONValueFree(server); return NULL; } after the fetching of server. Whether "vxhs" will be unique enough remains to be thought about too. and with all the above done correctly, cleanup: is unnecessary s/ == true// Comparatively to chardev, see qemuDomainPrepareChardevSourceTLS Don't believe this will be necessary, the default being if not configured as "NO" then if the system configures it, it will be configured used, so no need to modify the domain to say you want it. I think we'll need to add that - especially if we ever consider hot plug/unplug. By setting this to true here requires that the environment that was configured will have the server verify the client as per the various verify descriptions in qemu.conf. There's some extraneous indent/spacing issues... s/cfg->vxhsTLS == false &&/!cfg->vxhsTLS/ And this begs the question why? It's the whole reason for a "default" environment. The error message is also way too long... Still this type of check is better left to *Prepare* type logic. The more recent "desire" in the qemu_command code is not fail on configuration errors, but fail only on command generation errors. Not convinced yet 'addTLS' is needed - need to do some more thinking on this. This has already been a day long exercise - so I'll stop at this patch. I think we can get everything through the TLS changes merged into libvirt upstream once 3.8.0 opens for business. Whether more is ready depends on how much progress we can make. John

On 08/29/2017 02:39 AM, Ashish Mittal wrote: This should also add a qemuxml2xmltest.c adjustment (similar to earlier) dir=/etc/pki/qemu these will be file.server.0.{host|port} as well as adding the .type Remove this and this John

On 08/29/2017 02:39 AM, Ashish Mittal wrote: Similar to previous alter to have "type":"tcp" and use 9999 for port. Although this does work as is. John

On 08/29/2017 02:39 AM, Ashish Mittal wrote: While this works as advertised, I'm not clear why it matters. It's something we can talk about/debate once I repost the series. I'll leave it in the next posting, but I'm not sure it's desired. John

On 08/29/2017 02:39 AM, Ashish Mittal wrote: dir=/etc/pki/qemu dir=/etc/pki/qemu But this points out a problem - you now have two objects using the same id "id=objvxhs_tls0". This would fail a real qemu start... So here's my suggestion - see that "drive-virtio-disk0" - let's make use of that and instead of "objvxhs_tls0" - this would become "objvirtio-disk0-tls" - it's generic. This of course goes back to patch 5 where the alias was first created... Need to alter the various outputs to include the file.server.0.type and then file.server.0 for host and port Remove... Remove... Remove... !!!! Same as other one!!! This is virtio-disk1, but still it's unnecessary, so it needs to be removed. Remove !! would be virtio-disk2, but remove it. As you can see - I've gone through everything now. While doing so I've been making changes to sources and patch order a bit. I'll clean that all up and post in a little while. I need to go through each patch and figure out what I changed in order to "call it out"... John