[libvirt] [PATCH v2 0/7] Support to use iscsi storage in domain conf
https://bugzilla.redhat.com/show_bug.cgi?id=957295 This is an update of Osier's original patch: https://www.redhat.com/archives/libvir-list/2013-June/msg00673.html I initialially reviewed the patches, but now own the case since Osier is still temporarily out. Changes in v2: * Patch 1 - no change * Update patch 2 to add the missing test and clean up text as I noted * Split patch 3 into two parts Part A: Just the creation of virDomainDiskSourceIsBlockType() - Fixed the code to handle my code review comment regarding checking srcpool too early (before BLOCK) and fix grammar issues noted Part B: Adding 'pooltype' and adjusting qemuTranslateDiskSourcePool() in order to handle the "mode='uri'" so that when the disk is added via qemu_command the data is available for add the disk as ISCSI * Patch 5 (previously patch 4) dealt with changes to security backends; however, I found that those were unnecessary since according to the docs in formatdomain.html in the devices section "(NB, for "volume" type disk, seclabel is only valid when the specified storage volume is of 'file' or 'block' type)." However, since Dan requested to have some tests added for network and volume disk types to the securityselinuxlabeltest, I took the liberty to add those even though the net result is they don't add labels which I guess is an appropriate check - that is that no label is added. NOTE: I tried to figure out a way to add a 'file' storage pool that labels could be attached to, but was unsuccessful. * Patch 6 & 7 (formerly 5 & 6) - no change. Now that I know more about the code the issue I had with 6 is a no-op John Ferlan (3): conf: Introduce virDomainDiskSourceIsBlockType qemu: Translate the iscsi pool/volume disk source tests: Add various network and volume definitions Osier Yang (4): storage_iscsi: Reflect the default target port conf: Introduce new XML tag "mode" for disk source conf: Ignore the volume type disk if its mode is "uri" qemu: Translate the volume type disk source before cgroup setting docs/formatdomain.html.in | 9 +- docs/schemas/domaincommon.rng | 8 ++ src/conf/domain_conf.c | 68 +++++++++++- src/conf/domain_conf.h | 26 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 20 +++- src/qemu/qemu_conf.c | 117 +++++++++++++++++---- src/qemu/qemu_process.c | 13 ++- src/storage/storage_backend_iscsi.c | 6 +- .../qemuxml2argv-disk-source-pool-mode.xml | 48 +++++++++ tests/qemuxml2xmltest.c | 1 + tests/securityselinuxlabeldata/netdisks.txt | 5 + tests/securityselinuxlabeldata/netdisks.xml | 58 ++++++++++ tests/securityselinuxlabeldata/voldisks.txt | 5 + tests/securityselinuxlabeldata/voldisks.xml | 45 ++++++++ tests/securityselinuxlabeltest.c | 2 + 16 files changed, 400 insertions(+), 32 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml create mode 100644 tests/securityselinuxlabeldata/netdisks.txt create mode 100644 tests/securityselinuxlabeldata/netdisks.xml create mode 100644 tests/securityselinuxlabeldata/voldisks.txt create mode 100644 tests/securityselinuxlabeldata/voldisks.xml -- 1.8.1.4
From: Osier Yang <jyang@redhat.com> The default port for iSCSI target is 3260, which should be reflected to the pool's def. --- src/storage/storage_backend_iscsi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index ba4f388..08ba2d2 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -44,12 +44,14 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE +#define ISCSI_DEFAULT_TARGET_PORT 3260 + static char * virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) { char *portal = NULL; const char *host; - int port = 3260; + int port; if (source->nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -60,6 +62,8 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) host = source->hosts[0].name; if (source->hosts[0].port != 0) port = source->hosts[0].port; + else + port = source->hosts[0].port = ISCSI_DEFAULT_TARGET_PORT; if (strchr(host, ':')) { ignore_value(virAsprintf(&portal, "[%s]:%d,1", host, port)); -- 1.8.1.4
On Thu, Jul 18, 2013 at 11:02:36AM -0400, John Ferlan wrote:
From: Osier Yang <jyang@redhat.com>
The default port for iSCSI target is 3260, which should be reflected to the pool's def. --- src/storage/storage_backend_iscsi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index ba4f388..08ba2d2 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -44,12 +44,14 @@
#define VIR_FROM_THIS VIR_FROM_STORAGE
+#define ISCSI_DEFAULT_TARGET_PORT 3260 + static char * virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) { char *portal = NULL; const char *host; - int port = 3260; + int port;
if (source->nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -60,6 +62,8 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) host = source->hosts[0].name; if (source->hosts[0].port != 0) port = source->hosts[0].port; + else + port = source->hosts[0].port = ISCSI_DEFAULT_TARGET_PORT;
I think this would read better as if (source->hosts[0].port == 0) source->hosts[0].port = ISCSI_DEFAULT_TARGET_PORT; and just remove use of the separate 'port' variable completely...
if (strchr(host, ':')) { ignore_value(virAsprintf(&portal, "[%s]:%d,1", host, port));
Just reference source->hosts[0].port here. 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 :|
From: Osier Yang <jyang@redhat.com> There are two ways to use a iSCSI LUN as disk source for qemu. * The LUN's path as it shows up on host, e.g. /dev/disk/by-path/ip-$ip:3260-iscsi-$iqn-fc18:iscsi.iscsi0-lun-1 * The libiscsi URI, e.g. iscsi://demo.org:6000/iqn.1992-01.com.example/1 For a "volume" type disk, if the specified "pool" is of iscsi type, we should support to use the LUN in either of above 2 ways. That's why to introduce a new XML tag "mode" for the disk source (libvirt should support iscsi pool with libiscsi, but it's another new feature, which should be done later). The "mode" can be either of "host" or "uri". Use "host" to indicate use of the LUN with the path as it shows up on host. Use "uri" to indicate to use it with the libiscsi URI (future patches may support to use network type libvirt storage too, e.g. Ceph, that's why to use a more general name "uri"). --- docs/formatdomain.html.in | 9 +++- docs/schemas/domaincommon.rng | 8 ++++ src/conf/domain_conf.c | 22 +++++++++- src/conf/domain_conf.h | 22 ++++++++++ .../qemuxml2argv-disk-source-pool-mode.xml | 48 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b1c3bfc..22435a2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1601,7 +1601,14 @@ <code>pool</code> and <code>volume</code>. Attribute <code>pool</code> specifies the name of storage pool (managed by libvirt) where the disk source resides, and attribute <code>volume</code> specifies the name of - storage volume (managed by libvirt) used as the disk source. + storage volume (managed by libvirt) used as the disk source. For a + "volume" type disk, if the underlying storage pool is "iscsi", attribute + <code>mode</code> (<span class="since">since 1.1.1</span>) can be used + to indicate how to represent the LUN as the disk source. The value + "host" indicates to use the LUN's path as it shows up on host, e.g. + /dev/disk/by-path/ip-10.11.12.9:3260-iscsi-iqn.2013-06.fc:iscsi.iscsi0-lun-1). + The value "uri" indicates to use the libiscsi URI, e.g. + file=iscsi://demo.org:6000/iqn.1992-01.com.example/1. <span class="since">Since 0.0.3; <code>type='dir'</code> since 0.7.5; <code>type='network'</code> since 0.8.7; <code>protocol='iscsi'</code> since 1.0.4; diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7a6852b..40702be 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1174,6 +1174,14 @@ <ref name="volName"/> </attribute> <optional> + <attribute name="mode"> + <choice> + <value>host</value> + <value>uri</value> + </choice> + </attribute> + </optional> + <optional> <ref name="startupPolicy"/> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 44be81e..06b44b1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -761,6 +761,11 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST, "default", "unmap", "ignore") +VIR_ENUM_IMPL(virDomainDiskSourcePoolMode, + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST, + "default", + "host", + "uri") #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE @@ -4645,10 +4650,12 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node, { char *pool = NULL; char *volume = NULL; + char *mode = NULL; int ret = -1; pool = virXMLPropString(node, "pool"); volume = virXMLPropString(node, "volume"); + mode = virXMLPropString(node, "mode"); /* CD-ROM and Floppy allows no source */ if (!pool && !volume) @@ -4664,6 +4671,14 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node, if (VIR_ALLOC(def->srcpool) < 0) goto cleanup; + if (mode && (def->srcpool->mode = + virDomainDiskSourcePoolModeTypeFromString(mode)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown source mode '%s' for volume type disk"), + mode); + goto cleanup; + } + def->srcpool->pool = pool; pool = NULL; def->srcpool->volume = volume; @@ -4674,6 +4689,7 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node, cleanup: VIR_FREE(pool); VIR_FREE(volume); + VIR_FREE(mode); return ret; } @@ -14157,9 +14173,13 @@ virDomainDiskSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_VOLUME: virBufferAddLit(buf, " <source"); - if (def->srcpool) + if (def->srcpool) { virBufferAsprintf(buf, " pool='%s' volume='%s'", def->srcpool->pool, def->srcpool->volume); + if (def->srcpool->mode) + virBufferAsprintf(buf, " mode='%s'", + virDomainDiskSourcePoolModeTypeToString(def->srcpool->mode)); + } if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b14afd9..c21d1e2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -652,11 +652,32 @@ struct _virDomainBlockIoTuneInfo { }; typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; +/* + * Used for volume "type" disk to indicate how to represent + * the disk source if the specified "pool" is of iscsi type. + */ +enum virDomainDiskSourcePoolMode { + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DEFAULT = 0, + + /* Use the path as it shows up on host, e.g. + * /dev/disk/by-path/ip-$ip-iscsi-$iqn:iscsi.iscsi-pool0-lun-1 + */ + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST, + + /* Use the URI. E.g. + * file=iscsi://demo.org:6000/iqn.1992-01.com.example/1. + */ + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI, + + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST +}; + typedef struct _virDomainDiskSourcePoolDef virDomainDiskSourcePoolDef; struct _virDomainDiskSourcePoolDef { char *pool; /* pool name */ char *volume; /* volume name */ int voltype; /* enum virStorageVolType, internal only */ + int mode; /* enum virDomainDiskSourcePoolMode */ }; typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; @@ -2554,6 +2575,7 @@ VIR_ENUM_DECL(virDomainDiskSecretType) VIR_ENUM_DECL(virDomainDeviceSGIO) VIR_ENUM_DECL(virDomainDiskTray) VIR_ENUM_DECL(virDomainDiskDiscard) +VIR_ENUM_DECL(virDomainDiskSourcePoolMode) VIR_ENUM_DECL(virDomainIoEventFd) VIR_ENUM_DECL(virDomainVirtioEventIdx) VIR_ENUM_DECL(virDomainDiskCopyOnRead) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml new file mode 100644 index 0000000..d44013e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml @@ -0,0 +1,48 @@ +<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='volume' device='cdrom'> + <source pool='blk-pool0' volume='blk-pool0-vol0' mode='host' startupPolicy='optional'> + <seclabel model='selinux' relabel='yes'> + <label>system_u:system_r:public_content_t:s0</label> + </seclabel> + </source> + <target dev='hda' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <disk type='volume' device='cdrom'> + <source pool='blk-pool0' volume='blk-pool0-vol1' mode='uri' startupPolicy='optional'> + <seclabel model='selinux' relabel='yes'> + <label>system_u:system_r:public_content_t:s0</label> + </seclabel> + </source> + <target dev='hda' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='2'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/idedisk.img'/> + <target dev='hdc' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='3'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='ide' index='1'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 76570c5..77cac3f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -264,6 +264,7 @@ mymain(void) DO_TEST("disk-scsi-disk-vpd"); DO_TEST("disk-source-pool"); + DO_TEST("disk-source-pool-mode"); DO_TEST("disk-drive-discard"); -- 1.8.1.4
On Thu, Jul 18, 2013 at 11:02:37AM -0400, John Ferlan wrote:
From: Osier Yang <jyang@redhat.com>
There are two ways to use a iSCSI LUN as disk source for qemu.
* The LUN's path as it shows up on host, e.g. /dev/disk/by-path/ip-$ip:3260-iscsi-$iqn-fc18:iscsi.iscsi0-lun-1
* The libiscsi URI, e.g. iscsi://demo.org:6000/iqn.1992-01.com.example/1
For a "volume" type disk, if the specified "pool" is of iscsi type, we should support to use the LUN in either of above 2 ways. That's why to introduce a new XML tag "mode" for the disk source (libvirt should support iscsi pool with libiscsi, but it's another new feature, which should be done later).
The "mode" can be either of "host" or "uri". Use "host" to indicate use of the LUN with the path as it shows up on host. Use "uri" to indicate to use it with the libiscsi URI (future patches may support to use network type libvirt storage too, e.g. Ceph, that's why to use a more general name "uri").
I'm thinking that the mode is really describing the way in which QEMU uses the iSCSI LUN. Either it access it via the host, or it accesses it directly. The fact that the direct access mode has a cli arg encoded as an URI is really an implementation detail of current QEMU cli syntax. So I'd suggest the "mode" accept either "host" or "direct" instead of "host" or "uri". 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 19/07/13 00:09, Daniel P. Berrange wrote:
On Thu, Jul 18, 2013 at 11:02:37AM -0400, John Ferlan wrote:
From: Osier Yang <jyang@redhat.com>
There are two ways to use a iSCSI LUN as disk source for qemu.
* The LUN's path as it shows up on host, e.g. /dev/disk/by-path/ip-$ip:3260-iscsi-$iqn-fc18:iscsi.iscsi0-lun-1
* The libiscsi URI, e.g. iscsi://demo.org:6000/iqn.1992-01.com.example/1
For a "volume" type disk, if the specified "pool" is of iscsi type, we should support to use the LUN in either of above 2 ways. That's why to introduce a new XML tag "mode" for the disk source (libvirt should support iscsi pool with libiscsi, but it's another new feature, which should be done later).
The "mode" can be either of "host" or "uri". Use "host" to indicate use of the LUN with the path as it shows up on host. Use "uri" to indicate to use it with the libiscsi URI (future patches may support to use network type libvirt storage too, e.g. Ceph, that's why to use a more general name "uri"). I'm thinking that the mode is really describing the way in which QEMU uses the iSCSI LUN. Either it access it via the host, or it accesses it directly. The fact that the direct access mode has a cli arg encoded as an URI is really an implementation detail of current QEMU cli syntax.
yes.
So I'd suggest the "mode" accept either "host" or "direct" instead of "host" or "uri".
agreed. i was not that sure about the name "uri" when making the patches actually, "direct" sounds better to me.
Daniel
Introduce a new helper to check if the disk source is of block type --- src/conf/domain_conf.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 5 ++--- src/qemu/qemu_conf.c | 23 +++++------------------ 5 files changed, 52 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 06b44b1..4f24ecf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -41,6 +41,7 @@ #include "virbuffer.h" #include "virlog.h" #include "nwfilter_conf.h" +#include "storage_conf.h" #include "virstoragefile.h" #include "virfile.h" #include "virbitmap.h" @@ -18334,3 +18335,43 @@ virDomainDiskDefGenSecurityLabelDef(const char *model) return seclabel; } + +/** + * virDomainDiskSourceIsBlockType: + * + * Check if the disk *source* is of block type. This just tries + * to check from the type of disk def, not to probe the underlying + * storage. + * + * Return true if its source is block type, or false otherwise. + */ +bool +virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) +{ + /* No reason to think the disk source is block type if + * the source is empty + */ + if (!def->src) + return false; + + if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) + return true; + + /* For volume types, check the srcpool. + * If it's a block type source pool, then it's possible + */ + if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->srcpool && + def->srcpool->voltype == VIR_STORAGE_VOL_BLOCK) { + /* We don't think the volume accessed by remote URI is + * block type source, since we can't/shouldn't manage it + * (e.g. set sgio=filtered|unfiltered for it) in libvirt. + */ + if (def->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && + def->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI) + return false; + + return true; + } + + return false; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c21d1e2..05400ce 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2710,4 +2710,7 @@ virDomainDefMaybeAddController(virDomainDefPtr def, char *virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps); +bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) + ATTRIBUTE_NONNULL(1); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fc4e750..047d2db 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -204,6 +204,7 @@ virDomainDiskProtocolTransportTypeToString; virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; +virDomainDiskSourceIsBlockType; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainEmulatorPinAdd; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71e37f3..591279b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -42,6 +42,7 @@ #include "domain_audit.h" #include "domain_conf.h" #include "snapshot_conf.h" +#include "storage_conf.h" #include "network/bridge_driver.h" #include "virnetdevtap.h" #include "base64.h" @@ -3490,9 +3491,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskProtocolTypeToString(disk->protocol)); goto error; } - } else if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK)) { + } else if (!virDomainDiskSourceIsBlockType(disk)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk device='lun' is only valid for block type disk source")); goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c91551f..4fa0055 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -52,6 +52,7 @@ #include "virfile.h" #include "virstring.h" #include "viratomic.h" +#include "storage_conf.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1160,12 +1161,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1271,12 +1267,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1366,12 +1357,8 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk; - if (!disk->src || - disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || + virDomainDiskSourceIsBlockType(disk)) return 0; path = disk->src; -- 1.8.1.4
On 07/18/2013 11:02 AM, John Ferlan wrote:
Introduce a new helper to check if the disk source is of block type --- src/conf/domain_conf.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 5 ++--- src/qemu/qemu_conf.c | 23 +++++------------------ 5 files changed, 52 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 06b44b1..4f24ecf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -41,6 +41,7 @@ #include "virbuffer.h" #include "virlog.h" #include "nwfilter_conf.h" +#include "storage_conf.h" #include "virstoragefile.h" #include "virfile.h" #include "virbitmap.h" @@ -18334,3 +18335,43 @@ virDomainDiskDefGenSecurityLabelDef(const char *model)
return seclabel; } + +/** + * virDomainDiskSourceIsBlockType: + * + * Check if the disk *source* is of block type. This just tries + * to check from the type of disk def, not to probe the underlying + * storage. + * + * Return true if its source is block type, or false otherwise. + */ +bool +virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) +{ + /* No reason to think the disk source is block type if + * the source is empty + */ + if (!def->src) + return false; + + if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) + return true; + + /* For volume types, check the srcpool. + * If it's a block type source pool, then it's possible + */ + if (def->type == VIR_DOMAIN_DISK_TYPE_VOLUME && def->srcpool && + def->srcpool->voltype == VIR_STORAGE_VOL_BLOCK) { + /* We don't think the volume accessed by remote URI is + * block type source, since we can't/shouldn't manage it + * (e.g. set sgio=filtered|unfiltered for it) in libvirt. + */ + if (def->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI && + def->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI) + return false;
UG... I messed up the split the above 7 lines have to go in the next patch... I also found out that I forgot to update my branch before sending and I have a couple of merges to handle... John
+ + return true; + } + + return false; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c21d1e2..05400ce 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2710,4 +2710,7 @@ virDomainDefMaybeAddController(virDomainDefPtr def,
char *virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps);
+bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def) + ATTRIBUTE_NONNULL(1); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fc4e750..047d2db 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -204,6 +204,7 @@ virDomainDiskProtocolTransportTypeToString; virDomainDiskProtocolTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; +virDomainDiskSourceIsBlockType; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainEmulatorPinAdd; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71e37f3..591279b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -42,6 +42,7 @@ #include "domain_audit.h" #include "domain_conf.h" #include "snapshot_conf.h" +#include "storage_conf.h" #include "network/bridge_driver.h" #include "virnetdevtap.h" #include "base64.h" @@ -3490,9 +3491,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskProtocolTypeToString(disk->protocol)); goto error; } - } else if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK)) { + } else if (!virDomainDiskSourceIsBlockType(disk)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("disk device='lun' is only valid for block type disk source")); goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c91551f..4fa0055 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -52,6 +52,7 @@ #include "virfile.h" #include "virstring.h" #include "viratomic.h" +#include "storage_conf.h" #include "configmake.h"
#define VIR_FROM_THIS VIR_FROM_QEMU @@ -1160,12 +1161,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk;
- if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1271,12 +1267,7 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk;
- if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (!disk->shared || !virDomainDiskSourceIsBlockType(disk)) return 0; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; @@ -1366,12 +1357,8 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) if (dev->type == VIR_DOMAIN_DEVICE_DISK) { disk = dev->data.disk;
- if (!disk->src || - disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || + virDomainDiskSourceIsBlockType(disk)) return 0;
path = disk->src;
The difference with already supported pool types (dir, fs, block) is: there are two modes for iscsi pool (or network pools in future), one can specify it either to use the volume target path (the path showed up on host) with mode='host', or to use the remote URI qemu supports (e.g. file=iscsi://example.org:6000/iqn.1992-01.com.example/1) with mode='uri'. For 'host' mode, it copies the volume target path into disk->src. For 'uri' mode, the corresponding info in the *one* pool source host def are copied to disk->hosts[0]. --- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 15 +++++++- src/qemu/qemu_conf.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 05400ce..82f74ca 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -677,6 +677,7 @@ struct _virDomainDiskSourcePoolDef { char *pool; /* pool name */ char *volume; /* volume name */ int voltype; /* enum virStorageVolType, internal only */ + int pooltype; /* enum virStoragePoolType, internal only */ int mode; /* enum virDomainDiskSourcePoolMode */ }; typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 591279b..b5c1400 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3204,7 +3204,20 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, "block type volume")); goto error; } - virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); + + if (disk->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI) { + if (disk->srcpool->mode == + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI) { + if (qemuBuildISCSIString(conn, disk, &opt) < 0) + goto error; + virBufferAddChar(&opt, ','); + } else if (disk->srcpool->mode == + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) { + virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); + } + } else { + virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); + } break; case VIR_STORAGE_VOL_FILE: virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4fa0055..8fb3cda 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1418,12 +1418,68 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver) return virAtomicIntInc(&driver->nextvmid); } +static int +qemuAddDiskISCSIUri(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ + int ret = -1; + char **tokens = NULL; + + /* iscsi pool only supports one host */ + def->nhosts = 1; + + if (VIR_ALLOC_N(def->hosts, def->nhosts) < 0) + goto cleanup; + + if (VIR_STRDUP(def->hosts[0].name, pooldef->source.hosts[0].name) < 0) + goto cleanup; + + if (virAsprintf(&def->hosts[0].port, "%d", + pooldef->source.hosts[0].port ? + pooldef->source.hosts[0].port : + 3260) < 0) + goto cleanup; + + /* iscsi volume has name like "unit:0:0:1" */ + if (!(tokens = virStringSplit(def->srcpool->volume, ":", 0))) + goto cleanup; + + if (virStringListLength(tokens) != 4) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected iscsi volume name '%s'"), + def->srcpool->volume); + goto cleanup; + } + + /* iscsi pool has only one source device path */ + if (virAsprintf(&def->src, "%s/%s", + pooldef->source.devices[0].path, + tokens[3]) < 0) + goto cleanup; + + /* Storage pool have not supported these 2 attributes yet, + * use the defaults. + */ + def->hosts[0].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; + def->hosts[0].socket = NULL; + + def->protocol = VIR_DOMAIN_DISK_PROTOCOL_ISCSI; + + ret = 0; + +cleanup: + virStringFreeList(tokens); + return ret; +} + int qemuTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def) { + virStoragePoolDefPtr pooldef = NULL; virStoragePoolPtr pool = NULL; virStorageVolPtr vol = NULL; + char *poolxml = NULL; virStorageVolInfo info; int ret = -1; @@ -1451,11 +1507,45 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, switch (info.type) { case VIR_STORAGE_VOL_FILE: - case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_DIR: if (!(def->src = virStorageVolGetPath(vol))) goto cleanup; break; + case VIR_STORAGE_VOL_BLOCK: + if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) + goto cleanup; + + if (!(pooldef = virStoragePoolDefParseString(poolxml))) + goto cleanup; + + if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("disk source mode is only valid when " + "storage pool is of iscsi type")); + goto cleanup; + } + + def->srcpool->pooltype = pooldef->type; + if (pooldef->type == VIR_STORAGE_POOL_ISCSI) { + /* Default to use the LUN's path on host */ + if (!def->srcpool->mode) + def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST; + + if (def->srcpool->mode == + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI) { + if (qemuAddDiskISCSIUri(def, pooldef) < 0) + goto cleanup; + } else if (def->srcpool->mode == + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) { + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + } + } else { + if (!(def->src = virStorageVolGetPath(vol))) + goto cleanup; + } + + break; case VIR_STORAGE_VOL_NETWORK: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Using network volume as disk source is not supported")); @@ -1467,5 +1557,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, cleanup: virStoragePoolFree(pool); virStorageVolFree(vol); + VIR_FREE(poolxml); + virStoragePoolDefFree(pooldef); return ret; } -- 1.8.1.4
On 18/07/13 23:02, John Ferlan wrote:
The difference with already supported pool types (dir, fs, block) is: there are two modes for iscsi pool (or network pools in future),
s/or network/or other network/,
one can specify it either to use the volume target path (the path showed up on host) with mode='host', or to use the remote URI qemu supports (e.g. file=iscsi://example.org:6000/iqn.1992-01.com.example/1) with mode='uri'.
For 'host' mode, it copies the volume target path into disk->src. For 'uri' mode, the corresponding info in the*one* pool source host def are copied to disk->hosts[0].
s/are/is/, ?
---
Although they produce no seclabel data, add some tests for coverage of various network and volume disk definitions --- tests/securityselinuxlabeldata/netdisks.txt | 5 +++ tests/securityselinuxlabeldata/netdisks.xml | 58 +++++++++++++++++++++++++++++ tests/securityselinuxlabeldata/voldisks.txt | 5 +++ tests/securityselinuxlabeldata/voldisks.xml | 45 ++++++++++++++++++++++ tests/securityselinuxlabeltest.c | 2 + 5 files changed, 115 insertions(+) create mode 100644 tests/securityselinuxlabeldata/netdisks.txt create mode 100644 tests/securityselinuxlabeldata/netdisks.xml create mode 100644 tests/securityselinuxlabeldata/voldisks.txt create mode 100644 tests/securityselinuxlabeldata/voldisks.xml diff --git a/tests/securityselinuxlabeldata/netdisks.txt b/tests/securityselinuxlabeldata/netdisks.txt new file mode 100644 index 0000000..b6bf95f --- /dev/null +++ b/tests/securityselinuxlabeldata/netdisks.txt @@ -0,0 +1,5 @@ +/nbd.raw; +/iscsi.raw; +/rbd.raw; +/sheepdog.raw; +/gluster.raw; diff --git a/tests/securityselinuxlabeldata/netdisks.xml b/tests/securityselinuxlabeldata/netdisks.xml new file mode 100644 index 0000000..ab5e964 --- /dev/null +++ b/tests/securityselinuxlabeldata/netdisks.xml @@ -0,0 +1,58 @@ +<domain type='kvm'> + <name>vm1</name> + <uuid>c7b3edbd-edaf-9455-926a-d65c16db1800</uuid> + <memory unit='KiB'>219200</memory> + <os> + <type arch='i686' machine='pc-1.0'>hvm</type> + <boot dev='cdrom'/> + </os> + <devices> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='nbd' file="/nbd.raw"> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example/1' file="/iscsi.raw"> + <host name='example.org' port='6000'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + <disk type='network'> + <driver name="qemu" type="raw"/> + <source protocol="rbd" name="image_name2" file="/rbd.raw"> + <host name="hostname" port="7000"/> + </source> + <target dev="hdb" bus="ide"/> + <auth username='myuser'> + <secret type='ceph' usage='mypassid'/> + </auth> + </disk> + <disk type='network'> + <driver name="qemu" type="raw"/> + <source protocol="sheepdog" name="image_name" file="/sheepdog.raw"> + <host name="hostname" port="7000"/> + </source> + <target dev="hdb" bus="ide"/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='gluster' name='Volume/Image' file='/gluster.raw'> + <host name='example.org' port='6000' transport='tcp'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> + <listen type='address' address='0.0.0.0'/> + </graphics> + </devices> + <seclabel model="selinux" type="dynamic" relabel="yes"> + <label>system_u:system_r:svirt_t:s0:c41,c264</label> + <imagelabel>system_u:object_r:svirt_image_t:s0:c41,c264</imagelabel> + </seclabel> +</domain> diff --git a/tests/securityselinuxlabeldata/voldisks.txt b/tests/securityselinuxlabeldata/voldisks.txt new file mode 100644 index 0000000..bf14e91 --- /dev/null +++ b/tests/securityselinuxlabeldata/voldisks.txt @@ -0,0 +1,5 @@ +/file.raw; +/disk.raw; +/host.raw; +/uri.raw; +/cdrom.raw; diff --git a/tests/securityselinuxlabeldata/voldisks.xml b/tests/securityselinuxlabeldata/voldisks.xml new file mode 100644 index 0000000..c490c4f --- /dev/null +++ b/tests/securityselinuxlabeldata/voldisks.xml @@ -0,0 +1,45 @@ +<domain type='kvm'> + <name>vm1</name> + <uuid>c7b3edbd-edaf-9455-926a-d65c16db1800</uuid> + <memory unit='KiB'>219200</memory> + <os> + <type arch='i686' machine='pc-1.0'>hvm</type> + <boot dev='cdrom'/> + </os> + <devices> + <disk type='volume' device='disk'> + <driver name='qemu' type='raw'/> + <source pool='dir-pool0' volume='dir-pool0-vol0' file='/file.raw'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='volume' device='disk'> + <driver name='qemu' type='raw'/> + <source pool='dir-pool0' volume='dir-pool0-vol0' mode='host' file='/host.raw'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='volume' device='disk'> + <driver name='qemu' type='raw'/> + <source pool='dir-pool0' volume='dir-pool0-vol0' mode='uri' file='/uri.raw'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='volume' device='disk'> + <driver name='qemu' type='raw'/> + <source pool='blk-pool0' volume='blk-pool0-vol0' file='/plain.raw'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='volume' device='cdrom'> + <driver name='qemu' type='raw'/> + <source pool='blk-pool0' volume='blk-pool0-vol1' file='/cdrom.raw'/> + <target dev='hda' bus='ide'/> + <readonly/> + </disk> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> + <listen type='address' address='0.0.0.0'/> + </graphics> + </devices> + <seclabel model="selinux" type="dynamic" relabel="yes"> + <label>system_u:system_r:svirt_t:s0:c41,c264</label> + <imagelabel>system_u:object_r:svirt_image_t:s0:c41,c264</imagelabel> + </seclabel> +</domain> diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index efe825a..8c88cfd 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -332,6 +332,8 @@ mymain(void) setcon((security_context_t)"system_r:system_u:libvirtd_t:s0:c0.c1023"); DO_TEST_LABELING("disks"); + DO_TEST_LABELING("netdisks"); + DO_TEST_LABELING("voldisks"); DO_TEST_LABELING("kernel"); DO_TEST_LABELING("chardev"); -- 1.8.1.4
From: Osier Yang <jyang@redhat.com> virDomainDiskDefForeachPath is not only used by the security setting helpers, also used by cgroup setting helpers, so this is to ignore the volume type disk with mode="uri" for cgroup setting. --- src/conf/domain_conf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4f24ecf..f040d0a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17533,7 +17533,10 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, size_t depth = 0; virStorageFileMetadata *tmp; - if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) + if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK || + (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && + disk->srcpool && + disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_URI)) return 0; if (iter(disk, disk->src, 0, opaque) < 0) -- 1.8.1.4
From: Osier Yang <jyang@redhat.com> The translation must be done before both of cgroup and security setting, otherwise since the disk source is not translated yet, it might be skipped on cgroup and security setting. --- src/qemu/qemu_process.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3d5e8f6..95572ee 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3593,6 +3593,14 @@ int qemuProcessStart(virConnectPtr conn, } hookData.nodemask = nodemask; + /* "volume" type disk's source must be translated before + * cgroup and security setting. + */ + for (i = 0; i < vm->def->ndisks; i++) { + if (qemuTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) + goto cleanup; + } + VIR_DEBUG("Setting up domain cgroup (if required)"); if (qemuSetupCgroup(driver, vm, nodemask) < 0) goto cleanup; @@ -3637,11 +3645,6 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } - for (i = 0; i < vm->def->ndisks; i++) { - if (qemuTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) - goto cleanup; - } - VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON, priv->qemuCaps, -- 1.8.1.4
participants (3)
-
Daniel P. Berrange -
John Ferlan -
Osier Yang