7:32 a.m.
An update to yesterday's posting:
https://www.redhat.com/archives/libvir-list/2013-July/msg01213.html
Changes in v3 over v2
* Update patch 1 per code review
* Use 'direct' instead of 'uri' and supporting documentaiton
* In patch 4, use qemuAddISCSIPoolSourceHost() instead of
qemuAddDiskISCSIUri. Also added a check in the function
that (pooldef->source.nhost != 1) to make sure we at least
have a host defined in the pool and of course that there's
only one.
John Ferlan (5):
storage_iscsi: Reflect the default target port
conf: Introduce new XML tag "mode" for disk source
conf: Introduce virDomainDiskSourceIsBlockType
qemu: Translate the iscsi pool/volume disk source
tests: Add various network and volume definitions
Osier Yang (2):
conf: Ignore the volume type disk if its mode is "direct"
qemu: Translate the volume type disk source before cgroup setting
docs/formatdomain.html.in | 11 +-
docs/schemas/domaincommon.rng | 8 ++
src/conf/domain_conf.c | 67 ++++++++++-
src/conf/domain_conf.h | 26 +++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 20 +++-
src/qemu/qemu_conf.c | 124 +++++++++++++++++----
src/qemu/qemu_process.c | 13 ++-
src/storage/storage_backend_iscsi.c | 19 ++--
.../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, 414 insertions(+), 39 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
7:32 a.m.
New subject: [libvirt] [PATCH v3 1/7] storage_iscsi: Reflect the default target port
Make sure default iSCSI target is 3260.
---
src/storage/storage_backend_iscsi.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
index ba4f388..54bcd14 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -44,12 +44,12 @@
#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;
if (source->nhost != 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -57,14 +57,17 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr source)
return NULL;
}
- host = source->hosts[0].name;
- if (source->hosts[0].port != 0)
- port = source->hosts[0].port;
+ if (source->hosts[0].port == 0)
+ source->hosts[0].port = ISCSI_DEFAULT_TARGET_PORT;
- if (strchr(host, ':')) {
- ignore_value(virAsprintf(&portal, "[%s]:%d,1", host, port));
+ if (strchr(source->hosts[0].name, ':')) {
+ ignore_value(virAsprintf(&portal, "[%s]:%d,1",
+ source->hosts[0].name,
+ source->hosts[0].port));
} else {
- ignore_value(virAsprintf(&portal, "%s:%d,1", host, port));
+ ignore_value(virAsprintf(&portal, "%s:%d,1",
+ source->hosts[0].name,
+ source->hosts[0].port));
}
return portal;
--
1.8.1.4
5 a.m.
New subject: [libvirt] [PATCH v3 1/7] storage_iscsi: Reflect the default target port
On Fri, Jul 19, 2013 at 08:32:27AM -0400, John Ferlan wrote:
Make sure default iSCSI target is 3260.
---
src/storage/storage_backend_iscsi.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
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 :|
7:32 a.m.
New subject: [libvirt] [PATCH v3 2/7] conf: Introduce new XML tag "mode" for disk source
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 from the storage pool source element host attribute, 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 "direct". Use
"host" to indicate
use of the LUN with the path as it shows up on host. Use "direct" to
indicate to use it with the source pool host URI (future patches may support
to use network type libvirt storage too, e.g. Ceph)
---
docs/formatdomain.html.in | 11 ++++-
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, 110 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..7601aaa 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1601,7 +1601,16 @@
<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 "direct" indicates to use the storage pool's
+ <code>source</code> element <code>host</code> attribute
as the
+ disk source for 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..745b959 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>direct</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 57cd9b1..419958f 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",
+ "direct")
#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 25dad16..982a94e 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 from the storage pool source element host attribute. E.g.
+ * file=iscsi://demo.org:6000/iqn.1992-01.com.example/1.
+ */
+ VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT,
+
+ 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..b907633
--- /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='direct' 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
5 a.m.
New subject: [libvirt] [PATCH v3 2/7] conf: Introduce new XML tag "mode" for disk source
On Fri, Jul 19, 2013 at 08:32:28AM -0400, John Ferlan wrote:
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 from the storage pool source element host attribute, 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 "direct". Use
"host" to indicate
use of the LUN with the path as it shows up on host. Use "direct" to
indicate to use it with the source pool host URI (future patches may support
to use network type libvirt storage too, e.g. Ceph)
---
docs/formatdomain.html.in | 11 ++++-
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, 110 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
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 :|
8:20 a.m.
New subject: [libvirt] [PATCH v3 2/7] conf: Introduce new XML tag "mode" for disk source
Il 19/07/2013 14:32, John Ferlan ha scritto:
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 from the storage pool source element host attribute, 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 "direct". Use
"host" to indicate
use of the LUN with the path as it shows up on host. Use "direct" to
indicate to use it with the source pool host URI (future patches may support
to use network type libvirt storage too, e.g. Ceph)
What is the default?
Should 'direct' be valid for non-network pools?
Paolo
---
docs/formatdomain.html.in | 11 ++++-
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, 110 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..7601aaa 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1601,7 +1601,16 @@
<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 "direct" indicates to use the storage pool's
+ <code>source</code> element <code>host</code> attribute
as the
+ disk source for 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..745b959 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>direct</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 57cd9b1..419958f 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",
+ "direct")
#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 25dad16..982a94e 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 from the storage pool source element host attribute. E.g.
+ * file=iscsi://demo.org:6000/iqn.1992-01.com.example/1.
+ */
+ VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT,
+
+ 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..b907633
--- /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='direct' 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");
7:32 a.m.
New subject: [libvirt] [PATCH v3 3/7] conf: Introduce virDomainDiskSourceIsBlockType
Introduce a new helper to check if the disk source is of block type
---
src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++++
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, 43 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 419958f..dddd55d 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"
@@ -18375,3 +18376,34 @@ virDomainDefFindDevice(virDomainDefPtr def,
return 0;
}
+
+/**
+ * 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) {
+ return true;
+ }
+ return false;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 982a94e..4db981d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2714,4 +2714,7 @@ int virDomainDefFindDevice(virDomainDefPtr def,
const char *devAlias,
virDomainDeviceDefPtr dev);
+bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def)
+ ATTRIBUTE_NONNULL(1);
+
#endif /* __DOMAIN_CONF_H */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7790ede..f13a8e2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -206,6 +206,7 @@ virDomainDiskProtocolTransportTypeToString;
virDomainDiskProtocolTypeToString;
virDomainDiskRemove;
virDomainDiskRemoveByName;
+virDomainDiskSourceIsBlockType;
virDomainDiskTypeFromString;
virDomainDiskTypeToString;
virDomainEmulatorPinAdd;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8c58a7c..5265f77 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"
@@ -3500,9 +3501,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 64214b9..332b9f2 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
@@ -867,12 +868,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;
@@ -978,12 +974,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;
@@ -1073,12 +1064,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
5:02 a.m.
New subject: [libvirt] [PATCH v3 3/7] conf: Introduce virDomainDiskSourceIsBlockType
On Fri, Jul 19, 2013 at 08:32:29AM -0400, John Ferlan wrote:
Introduce a new helper to check if the disk source is of block type
---
src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++++
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, 43 insertions(+), 21 deletions(-)
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 :|
7:32 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
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='direct'.
For 'host' mode, it copies the volume target path into disk->src. For
'direct' mode, the corresponding info in the *one* pool source host def
is copied to disk->hosts[0].
---
src/conf/domain_conf.c | 8 ++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 15 ++++++-
src/qemu/qemu_conf.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 123 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dddd55d..231edb5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18403,6 +18403,14 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def)
*/
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_DIRECT)
+ return false;
+
return true;
}
return false;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4db981d..327f3f9 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 5265f77..4e4ee9e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3214,7 +3214,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_DIRECT) {
+ 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 332b9f2..6e6163f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1125,12 +1125,75 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
return virAtomicIntInc(&driver->nextvmid);
}
+static int
+qemuAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
+ virStoragePoolDefPtr pooldef)
+{
+ int ret = -1;
+ char **tokens = NULL;
+
+ /* Only support one host */
+ if (pooldef->source.nhost != 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Expected exactly 1 host for the storage pool"));
+ goto cleanup;
+ }
+
+ /* 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;
@@ -1158,11 +1221,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_DIRECT) {
+ if (qemuAddISCSIPoolSourceHost(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"));
@@ -1174,5 +1271,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
cleanup:
virStoragePoolFree(pool);
virStorageVolFree(vol);
+ VIR_FREE(poolxml);
+ virStoragePoolDefFree(pooldef);
return ret;
}
--
1.8.1.4
5:30 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On 19/07/13 20:32, 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),
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='direct'.
For 'host' mode, it copies the volume target path into disk->src. For
'direct' mode, the corresponding info in the *one* pool source host def
is copied to disk->hosts[0].
---
src/conf/domain_conf.c | 8 ++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 15 ++++++-
src/qemu/qemu_conf.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 123 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dddd55d..231edb5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18403,6 +18403,14 @@ virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def)
*/
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_DIRECT)
+ return false;
+
return true;
}
return false;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4db981d..327f3f9 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 5265f77..4e4ee9e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3214,7 +3214,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_DIRECT) {
+ 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 332b9f2..6e6163f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1125,12 +1125,75 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
return virAtomicIntInc(&driver->nextvmid);
}
+static int
+qemuAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
+ virStoragePoolDefPtr pooldef)
nice abstraction. it's good other network pools' support in future.
+{
+ int ret = -1;
+ char **tokens = NULL;
+
+ /* Only support one host */
+ if (pooldef->source.nhost != 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Expected exactly 1 host for the storage pool"));
+ goto cleanup;
+ }
+
+ /* 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;
@@ -1158,11 +1221,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_DIRECT) {
+ if (qemuAddISCSIPoolSourceHost(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"));
@@ -1174,5 +1271,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
cleanup:
virStoragePoolFree(pool);
virStorageVolFree(vol);
+ VIR_FREE(poolxml);
+ virStoragePoolDefFree(pooldef);
return ret;
}
though i'm the original author, but it's long time and there are
changes.
ACK.
8:22 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 22/07/2013 12:30, Osier Yang ha scritto:
+ 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_DIRECT) {
+ if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0)
+ goto cleanup;
+ } else if (def->srcpool->mode ==
+ VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
+ if (!(def->src = virStorageVolGetPath(vol)))
+ goto cleanup;
+ }
Ok, this answers my question. :)
I think the default mode should be direct, because otherwise things such
as persistent reservations do not work.
Paolo
8:26 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 03:22:54PM +0200, Paolo Bonzini wrote:
Il 22/07/2013 12:30, Osier Yang ha scritto:
> + 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_DIRECT) {
> + if (qemuAddISCSIPoolSourceHost(def, pooldef) < 0)
> + goto cleanup;
> + } else if (def->srcpool->mode ==
> + VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
> + if (!(def->src = virStorageVolGetPath(vol)))
> + goto cleanup;
> + }
Ok, this answers my question. :)
I think the default mode should be direct, because otherwise things such
as persistent reservations do not work.
No, the default has to be host mode, because that is the only mode that
is guaranteed to be usable with any QEMU. The direct mode requires a
QEMU that is new enough, and a distro to have enabled it. We can't
rely on that as a default choice.
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 :|
8:29 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 15:26, Daniel P. Berrange ha scritto:
> >
> > Ok, this answers my question. :)
> >
> > I think the default mode should be direct, because otherwise things such
> > as persistent reservations do not work.
No, the default has to be host mode, because that is the only mode that
is guaranteed to be usable with any QEMU. The direct mode requires a
QEMU that is new enough, and a distro to have enabled it. We can't
rely on that as a default choice.
Volume sources are also new enough that you can assume a good QEMU.
Host mode for iSCSI is broken. It also doesn't reconnect well if you
have a network problem, because after a LUN rescan the inode may change.
Paolo
8:36 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote:
Il 23/07/2013 15:26, Daniel P. Berrange ha scritto:
>> >
>> > Ok, this answers my question. :)
>> >
>> > I think the default mode should be direct, because otherwise things such
>> > as persistent reservations do not work.
> No, the default has to be host mode, because that is the only mode that
> is guaranteed to be usable with any QEMU. The direct mode requires a
> QEMU that is new enough, and a distro to have enabled it. We can't
> rely on that as a default choice.
Volume sources are also new enough that you can assume a good QEMU.
No we can't assume that. New libvirt is frequently used with old QEMU
and we want to have good default behaviour there.
Host mode for iSCSI is broken. It also doesn't reconnect well if
you
have a network problem, because after a LUN rescan the inode may change.
That's a much smaller level of brokeness, than if QEMU doesn't support
the iscsi block protocol & thus the default configuration won't even
boot.
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 :|
8:52 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 15:36, Daniel P. Berrange ha scritto:
On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote:
> Il 23/07/2013 15:26, Daniel P. Berrange ha scritto:
>>>>
>>>> Ok, this answers my question. :)
>>>>
>>>> I think the default mode should be direct, because otherwise things such
>>>> as persistent reservations do not work.
>> No, the default has to be host mode, because that is the only mode that
>> is guaranteed to be usable with any QEMU. The direct mode requires a
>> QEMU that is new enough, and a distro to have enabled it. We can't
>> rely on that as a default choice.
>
> Volume sources are also new enough that you can assume a good QEMU.
No we can't assume that. New libvirt is frequently used with old QEMU
and we want to have good default behaviour there.
New libvirt, yes. But can't new libvirt features also assume a new QEMU
if the old one causes more trouble than anything?
> Host mode for iSCSI is broken. It also doesn't reconnect
well if you
> have a network problem, because after a LUN rescan the inode may change.
That's a much smaller level of brokeness, than if QEMU doesn't support
the iscsi block protocol & thus the default configuration won't even
boot.
Failure to reconnect is not part of the brokenness. :)
If you use host mode and the guest issues reservation commands, you may
get data corruption. That is the brokenness.
Another difference is that direct mode doesn't require the pool to be
started before starting the domain. For authenticated iSCSI LUNs, this
is better because you cannot autostart authenticated iSCSI pools.
Perhaps the default could be specified in a configuration file (and the
default should be the safe one).
Paolo
9:14 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 03:52:56PM +0200, Paolo Bonzini wrote:
Il 23/07/2013 15:36, Daniel P. Berrange ha scritto:
> On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote:
>> Il 23/07/2013 15:26, Daniel P. Berrange ha scritto:
>>>>>
>>>>> Ok, this answers my question. :)
>>>>>
>>>>> I think the default mode should be direct, because otherwise things
such
>>>>> as persistent reservations do not work.
>>> No, the default has to be host mode, because that is the only mode that
>>> is guaranteed to be usable with any QEMU. The direct mode requires a
>>> QEMU that is new enough, and a distro to have enabled it. We can't
>>> rely on that as a default choice.
>>
>> Volume sources are also new enough that you can assume a good QEMU.
>
> No we can't assume that. New libvirt is frequently used with old QEMU
> and we want to have good default behaviour there.
New libvirt, yes. But can't new libvirt features also assume a new QEMU
if the old one causes more trouble than anything?
I've no problem with new features requiring new QEMU but that's not
the issue here. This is a question about what the default configuration
should be. For default configuration we aim for maximum compatibility
not the latest possible features.
>> Host mode for iSCSI is broken. It also doesn't
reconnect well if you
>> have a network problem, because after a LUN rescan the inode may change.
>
> That's a much smaller level of brokeness, than if QEMU doesn't support
> the iscsi block protocol & thus the default configuration won't even
> boot.
Failure to reconnect is not part of the brokenness. :)
If you use host mode and the guest issues reservation commands, you may
get data corruption. That is the brokenness.
So be it, that's been the case for every version of libvirt and QEMU
in existance up until iscsi protocol support was added, so it is not
a new issue. That doesn't justify choosing a default that is guaranteed
to not work at all.
Another difference is that direct mode doesn't require the pool
to be
started before starting the domain. For authenticated iSCSI LUNs, this
is better because you cannot autostart authenticated iSCSI pools.
Autostarting of authenticated pools is simply a bug to be fixed.
Perhaps the default could be specified in a configuration file (and the
default should be the safe one).
No, that is even worse because now the default is not predictable..
We simply default to host mode and if applications want to use the
other mode they can configure the XML as desired.
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 :|
10:03 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
On Tue, Jul 23, 2013 at 03:52:56PM +0200, Paolo Bonzini wrote:
> Il 23/07/2013 15:36, Daniel P. Berrange ha scritto:
>> On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote:
>>> Il 23/07/2013 15:26, Daniel P. Berrange ha scritto:
>>>>>>
>>>>>> Ok, this answers my question. :)
>>>>>>
>>>>>> I think the default mode should be direct, because otherwise
things such
>>>>>> as persistent reservations do not work.
>>>> No, the default has to be host mode, because that is the only mode that
>>>> is guaranteed to be usable with any QEMU. The direct mode requires a
>>>> QEMU that is new enough, and a distro to have enabled it. We can't
>>>> rely on that as a default choice.
>>>
>>> Volume sources are also new enough that you can assume a good QEMU.
>>
>> No we can't assume that. New libvirt is frequently used with old QEMU
>> and we want to have good default behaviour there.
>
> New libvirt, yes. But can't new libvirt features also assume a new QEMU
> if the old one causes more trouble than anything?
I've no problem with new features requiring new QEMU but that's not
the issue here. This is a question about what the default configuration
should be. For default configuration we aim for maximum compatibility
not the latest possible features.
Shouldn't default configuration be simply "what makes the most sense"?
> If you use host mode and the guest issues reservation commands,
you may
> get data corruption. That is the brokenness.
So be it, that's been the case for every version of libvirt and QEMU
in existance up until iscsi protocol support was added, so it is not
a new issue. That doesn't justify choosing a default that is guaranteed
to not work at all.
I started adding iSCSI support to ensure that iSCSI LUNs would work.
This makes them _not_ work by default. It makes me regret asking to add
support for volumes as a disk source.
Paolo
10:35 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
> > Perhaps the default could be specified in a configuration
file (and the
> > default should be the safe one).
No, that is even worse because now the default is not predictable..
We simply default to host mode and if applications want to use the
other mode they can configure the XML as desired.
Can we just forbid mode='default' for iSCSI and force the user to
specify host vs. direct?
At least the error message will be nice and independent of the QEMU version.
Paolo
11:01 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
>> > Perhaps the default could be specified in a configuration file (and the
>> > default should be the safe one).
> No, that is even worse because now the default is not predictable..
>
> We simply default to host mode and if applications want to use the
> other mode they can configure the XML as desired.
Can we just forbid mode='default' for iSCSI and force the user to
specify host vs. direct?
That would mean that apps cannot simply configure a guest volume
without first checking to find out what type of pool it is, and
then specifying this extra arg for iSCSI. IMHO the value of the
<volume> XML is that you don't have to know anything about the
pool to be able to configure it - we're completely decoupled.
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 :|
11:18 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
> Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
>>>> Perhaps the default could be specified in a configuration file (and the
>>>> default should be the safe one).
>> No, that is even worse because now the default is not predictable..
>>
>> We simply default to host mode and if applications want to use the
>> other mode they can configure the XML as desired.
>
> Can we just forbid mode='default' for iSCSI and force the user to
> specify host vs. direct?
That would mean that apps cannot simply configure a guest volume
without first checking to find out what type of pool it is, and
then specifying this extra arg for iSCSI. IMHO the value of the
<volume> XML is that you don't have to know anything about the
pool to be able to configure it - we're completely decoupled.
Thinking more about it, it would only be needed for <disk type='volume'
device='lun'>. And for that case, some knowledge of the pool is
necessary anyway (for one thing, it won't work with filesystem or LVM
pools). So if we could forbid mode='default' for that case only, it
would be enough as far as I'm concernde.
Paolo
11:47 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On 07/23/2013 12:18 PM, Paolo Bonzini wrote:
Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
> On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
>> Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
>>>>> Perhaps the default could be specified in a configuration file (and
the
>>>>> default should be the safe one).
>>> No, that is even worse because now the default is not predictable..
>>>
>>> We simply default to host mode and if applications want to use the
>>> other mode they can configure the XML as desired.
>>
>> Can we just forbid mode='default' for iSCSI and force the user to
>> specify host vs. direct?
>
> That would mean that apps cannot simply configure a guest volume
> without first checking to find out what type of pool it is, and
> then specifying this extra arg for iSCSI. IMHO the value of the
> <volume> XML is that you don't have to know anything about the
> pool to be able to configure it - we're completely decoupled.
Thinking more about it, it would only be needed for <disk type='volume'
device='lun'>. And for that case, some knowledge of the pool is
necessary anyway (for one thing, it won't work with filesystem or LVM
pools). So if we could forbid mode='default' for that case only, it
would be enough as far as I'm concernde.
Paolo
Using "default" in the mode field would result in the following XML
error message (I just quickly changed a test to prove the point):
121) QEMU XML-2-XML disk-source-pool-mode
... libvirt: Domain Config error : XML error: unknown source mode
'default' for volume type disk
FAILED
The XML parsing code only looks for "mode='direct'" or
"mode='host'". If
"mode" isn't present in the XML, that's when that default comes into
play. Since 'mode' is new there could be configurations where its not in
an XML file, thus a 0 (zero e.g. "default") value is provided for the field.
Once the XML is parsed we still needed a default when it's going to be
added, thus the "magic" to set the default to HOST is in qemu_conf.c in
qemuTranslateDiskSourcePool():
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;
I think this answers your primary concern - correct?
John
12:11 p.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 18:47, John Ferlan ha scritto:
On 07/23/2013 12:18 PM, Paolo Bonzini wrote:
> Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
>> On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
>>> Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
>>>>>> Perhaps the default could be specified in a configuration file
(and the
>>>>>> default should be the safe one).
>>>> No, that is even worse because now the default is not predictable..
>>>>
>>>> We simply default to host mode and if applications want to use the
>>>> other mode they can configure the XML as desired.
>>>
>>> Can we just forbid mode='default' for iSCSI and force the user to
>>> specify host vs. direct?
>>
>> That would mean that apps cannot simply configure a guest volume
>> without first checking to find out what type of pool it is, and
>> then specifying this extra arg for iSCSI. IMHO the value of the
>> <volume> XML is that you don't have to know anything about the
>> pool to be able to configure it - we're completely decoupled.
>
> Thinking more about it, it would only be needed for <disk type='volume'
> device='lun'>. And for that case, some knowledge of the pool is
> necessary anyway (for one thing, it won't work with filesystem or LVM
> pools). So if we could forbid mode='default' for that case only, it
> would be enough as far as I'm concernde.
Using "default" in the mode field would result in the following XML
error message (I just quickly changed a test to prove the point):
121) QEMU XML-2-XML disk-source-pool-mode
... libvirt: Domain Config error : XML error: unknown source mode
'default' for volume type disk
FAILED
Sorry, by mode='default' I really meant "no mode at all" (I was under
the false impression that you could also specify a mode='default' with
the same effect as no mode at all).
The XML parsing code only looks for "mode='direct'"
or "mode='host'". If
"mode" isn't present in the XML, that's when that default comes into
play. Since 'mode' is new there could be configurations where its not in
an XML file, thus a 0 (zero e.g. "default") value is provided for the field.
Once the XML is parsed we still needed a default when it's going to be
added, thus the "magic" to set the default to HOST is in qemu_conf.c in
qemuTranslateDiskSourcePool():
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;
I think this answers your primary concern - correct?
No, my concern is that mode='host' is a bad default for the device='lun'
case.
Paolo
10:12 p.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On 24/07/13 01:11, Paolo Bonzini wrote:
Il 23/07/2013 18:47, John Ferlan ha scritto:
> On 07/23/2013 12:18 PM, Paolo Bonzini wrote:
>> Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
>>> On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
>>>> Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
>>>>>>> Perhaps the default could be specified in a configuration
file (and the
>>>>>>> default should be the safe one).
>>>>> No, that is even worse because now the default is not predictable..
>>>>>
>>>>> We simply default to host mode and if applications want to use the
>>>>> other mode they can configure the XML as desired.
>>>> Can we just forbid mode='default' for iSCSI and force the user
to
>>>> specify host vs. direct?
>>> That would mean that apps cannot simply configure a guest volume
>>> without first checking to find out what type of pool it is, and
>>> then specifying this extra arg for iSCSI. IMHO the value of the
>>> <volume> XML is that you don't have to know anything about the
>>> pool to be able to configure it - we're completely decoupled.
>> Thinking more about it, it would only be needed for <disk
type='volume'
>> device='lun'>. And for that case, some knowledge of the pool is
>> necessary anyway (for one thing, it won't work with filesystem or LVM
>> pools). So if we could forbid mode='default' for that case only, it
>> would be enough as far as I'm concernde.
> Using "default" in the mode field would result in the following XML
> error message (I just quickly changed a test to prove the point):
>
> 121) QEMU XML-2-XML disk-source-pool-mode
> ... libvirt: Domain Config error : XML error: unknown source mode
> 'default' for volume type disk
> FAILED
Sorry, by mode='default' I really meant "no mode at all" (I was under
the false impression that you could also specify a mode='default' with
the same effect as no mode at all).
> The XML parsing code only looks for "mode='direct'" or
"mode='host'". If
> "mode" isn't present in the XML, that's when that default comes
into
> play. Since 'mode' is new there could be configurations where its not in
> an XML file, thus a 0 (zero e.g. "default") value is provided for the
field.
>
> Once the XML is parsed we still needed a default when it's going to be
> added, thus the "magic" to set the default to HOST is in qemu_conf.c in
> qemuTranslateDiskSourcePool():
>
>
> 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;
>
>
> I think this answers your primary concern - correct?
No, my concern is that mode='host' is a bad default for the device='lun'
case.
after going through the thread......
i think the point is the relationship between the "host" and "direct"
mode.
if users used the "host" mode in the past, but most of them suffered from
the persistent reservatioins, and "direct" mode is the answer for them, then
i indent to agree with paolo that defaulting to "direct" is better.
if they used the "host" mode broadly, but persistent reservation is just
requirement of a small part of the users. then i don't see any problem to
default to "host" mode.
i.e. the point is if "direct" mode is the answer for "host" mode.
but anyway, we provide good enough documentations, so if one wants to
use "direct" mode, he can still change the xml to use it, any big problems
here? though defaulting to "direct" is more convenient for these users.
osier
4:54 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Wed, Jul 24, 2013 at 11:12:18AM +0800, Osier Yang wrote:
On 24/07/13 01:11, Paolo Bonzini wrote:
>Il 23/07/2013 18:47, John Ferlan ha scritto:
>>On 07/23/2013 12:18 PM, Paolo Bonzini wrote:
>>>Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
>>>>On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
>>>>>Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
>>>>>>>>Perhaps the default could be specified in a configuration
file (and the
>>>>>>>>default should be the safe one).
>>>>>>No, that is even worse because now the default is not
predictable..
>>>>>>
>>>>>>We simply default to host mode and if applications want to use
the
>>>>>>other mode they can configure the XML as desired.
>>>>>Can we just forbid mode='default' for iSCSI and force the
user to
>>>>>specify host vs. direct?
>>>>That would mean that apps cannot simply configure a guest volume
>>>>without first checking to find out what type of pool it is, and
>>>>then specifying this extra arg for iSCSI. IMHO the value of the
>>>><volume> XML is that you don't have to know anything about the
>>>>pool to be able to configure it - we're completely decoupled.
>>>Thinking more about it, it would only be needed for <disk
type='volume'
>>>device='lun'>. And for that case, some knowledge of the pool is
>>>necessary anyway (for one thing, it won't work with filesystem or LVM
>>>pools). So if we could forbid mode='default' for that case only, it
>>>would be enough as far as I'm concernde.
>>Using "default" in the mode field would result in the following XML
>>error message (I just quickly changed a test to prove the point):
>>
>>121) QEMU XML-2-XML disk-source-pool-mode
>>... libvirt: Domain Config error : XML error: unknown source mode
>>'default' for volume type disk
>>FAILED
>Sorry, by mode='default' I really meant "no mode at all" (I was
under
>the false impression that you could also specify a mode='default' with
>the same effect as no mode at all).
>
>>The XML parsing code only looks for "mode='direct'" or
"mode='host'". If
>>"mode" isn't present in the XML, that's when that default comes
into
>>play. Since 'mode' is new there could be configurations where its not in
>>an XML file, thus a 0 (zero e.g. "default") value is provided for the
field.
>>
>>Once the XML is parsed we still needed a default when it's going to be
>>added, thus the "magic" to set the default to HOST is in qemu_conf.c
in
>>qemuTranslateDiskSourcePool():
>>
>>
>> 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;
>>
>>
>>I think this answers your primary concern - correct?
>No, my concern is that mode='host' is a bad default for the
device='lun'
>case.
>
after going through the thread......
i think the point is the relationship between the "host" and "direct"
mode.
if users used the "host" mode in the past, but most of them suffered from
the persistent reservatioins, and "direct" mode is the answer for them, then
i indent to agree with paolo that defaulting to "direct" is better.
if they used the "host" mode broadly, but persistent reservation is just
requirement of a small part of the users. then i don't see any problem to
default to "host" mode.
i.e. the point is if "direct" mode is the answer for "host" mode.
but anyway, we provide good enough documentations, so if one wants to
use "direct" mode, he can still change the xml to use it, any big problems
here? though defaulting to "direct" is more convenient for these users.
Defaulting to "direct" is *NOT* more convenient for all users, only
those who happen to have a new enough QEMU, from a vendor who actually
decided to compile in the iSCSI support. If they're lacking such a QEMU
then the "direct" mode will prevent the default config from even starting.
This can never be an acceptable default choice. mode="host", while it huas
some limitations in features, is the only option that is capable of working
with any QEMU
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 :|
5:25 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 24/07/2013 11:54, Daniel P. Berrange ha scritto:
> but anyway, we provide good enough documentations, so if one
wants to
> use "direct" mode, he can still change the xml to use it, any big problems
> here? though defaulting to "direct" is more convenient for these users.
Defaulting to "direct" is *NOT* more convenient for all users, only
those who happen to have a new enough QEMU, from a vendor who actually
decided to compile in the iSCSI support. If they're lacking such a QEMU
then the "direct" mode will prevent the default config from even starting.
This can never be an acceptable default choice. mode="host", while it huas
some limitations in features, is the only option that is capable of working
with any QEMU
s/limitations in features/possibility of wreaking havoc to clusters/
Didn't you "break features that worked" when you started forbidding
migration unless cache=none? This time there is even hardly a feature
that worked, because the feature is new.
Paolo
5:31 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Wed, Jul 24, 2013 at 12:25:17PM +0200, Paolo Bonzini wrote:
Il 24/07/2013 11:54, Daniel P. Berrange ha scritto:
>> but anyway, we provide good enough documentations, so if one wants to
>> use "direct" mode, he can still change the xml to use it, any big
problems
>> here? though defaulting to "direct" is more convenient for these
users.
>
> Defaulting to "direct" is *NOT* more convenient for all users, only
> those who happen to have a new enough QEMU, from a vendor who actually
> decided to compile in the iSCSI support. If they're lacking such a QEMU
> then the "direct" mode will prevent the default config from even
starting.
> This can never be an acceptable default choice. mode="host", while it
huas
> some limitations in features, is the only option that is capable of working
> with any QEMU
s/limitations in features/possibility of wreaking havoc to clusters/
This isn't a new issue. People have been using KVM with iSCSI in host
mode for as long as KVM has existed and the sky hasn't fallen in
clusters. As long as it is a documented limitation, that admins can
address with a config setting if desired, I don't see that this is
suddenly something we must fix at the cost of breaking the default
config for QEMU's without iSCSI support.
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 :|
5:37 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 24/07/2013 12:31, Daniel P. Berrange ha scritto:
On Wed, Jul 24, 2013 at 12:25:17PM +0200, Paolo Bonzini wrote:
> Il 24/07/2013 11:54, Daniel P. Berrange ha scritto:
>>> but anyway, we provide good enough documentations, so if one wants to
>>> use "direct" mode, he can still change the xml to use it, any big
problems
>>> here? though defaulting to "direct" is more convenient for these
users.
>>
>> Defaulting to "direct" is *NOT* more convenient for all users, only
>> those who happen to have a new enough QEMU, from a vendor who actually
>> decided to compile in the iSCSI support. If they're lacking such a QEMU
>> then the "direct" mode will prevent the default config from even
starting.
>> This can never be an acceptable default choice. mode="host", while it
huas
>> some limitations in features, is the only option that is capable of working
>> with any QEMU
>
> s/limitations in features/possibility of wreaking havoc to clusters/
This isn't a new issue. People have been using KVM with iSCSI in host
mode for as long as KVM has existed and the sky hasn't fallen in
clusters. As long as it is a documented limitation, that admins can
address with a config setting if desired, I don't see that this is
suddenly something we must fix at the cost of breaking the default
config for QEMU's without iSCSI support.
Understood, that's why I suggested not having a default config at all
for type='lun' devices.
Also, can libvirt at least detect using a volume on a pool that wasn't
started, and fail unless mode='direct'?
Paolo
5:39 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Wed, Jul 24, 2013 at 12:37:10PM +0200, Paolo Bonzini wrote:
Il 24/07/2013 12:31, Daniel P. Berrange ha scritto:
> On Wed, Jul 24, 2013 at 12:25:17PM +0200, Paolo Bonzini wrote:
>> Il 24/07/2013 11:54, Daniel P. Berrange ha scritto:
>>>> but anyway, we provide good enough documentations, so if one wants to
>>>> use "direct" mode, he can still change the xml to use it, any
big problems
>>>> here? though defaulting to "direct" is more convenient for
these users.
>>>
>>> Defaulting to "direct" is *NOT* more convenient for all users,
only
>>> those who happen to have a new enough QEMU, from a vendor who actually
>>> decided to compile in the iSCSI support. If they're lacking such a QEMU
>>> then the "direct" mode will prevent the default config from even
starting.
>>> This can never be an acceptable default choice. mode="host", while
it huas
>>> some limitations in features, is the only option that is capable of working
>>> with any QEMU
>>
>> s/limitations in features/possibility of wreaking havoc to clusters/
>
> This isn't a new issue. People have been using KVM with iSCSI in host
> mode for as long as KVM has existed and the sky hasn't fallen in
> clusters. As long as it is a documented limitation, that admins can
> address with a config setting if desired, I don't see that this is
> suddenly something we must fix at the cost of breaking the default
> config for QEMU's without iSCSI support.
Understood, that's why I suggested not having a default config at all
for type='lun' devices.
As I said before, that causes apps to have to do special handling
for iSCSI pools, which is not something we want.
Also, can libvirt at least detect using a volume on a pool that
wasn't
started, and fail unless mode='direct'?
It should always fail to start the guest if the pool is not started,
regardless of the mode used.
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 :|
5:41 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 24/07/2013 12:39, Daniel P. Berrange ha scritto:
On Wed, Jul 24, 2013 at 12:37:10PM +0200, Paolo Bonzini wrote:
> Il 24/07/2013 12:31, Daniel P. Berrange ha scritto:
>> breaking the default config for QEMU's without iSCSI support.
>
> Understood, that's why I suggested not having a default config at all
> for type='lun' devices.
As I said before, that causes apps to have to do special handling
for iSCSI pools, which is not something we want.
This is not about iSCSI pools, it is about type='lun' devices.
And as I said before, knowing the kind of pool is something you have to
do anyway for type='lun' devices, because they won't work if the
underlying pool is filesystem- or LVM-based.
> Also, can libvirt at least detect using a volume on a pool that
wasn't
> started, and fail unless mode='direct'?
It should always fail to start the guest if the pool is not started,
regardless of the mode used.
Another point of mode='direct' is that the pool need not be started.
Paolo
5:46 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Wed, Jul 24, 2013 at 12:41:49PM +0200, Paolo Bonzini wrote:
Il 24/07/2013 12:39, Daniel P. Berrange ha scritto:
> On Wed, Jul 24, 2013 at 12:37:10PM +0200, Paolo Bonzini wrote:
>> Il 24/07/2013 12:31, Daniel P. Berrange ha scritto:
>>> breaking the default config for QEMU's without iSCSI support.
>>
>> Understood, that's why I suggested not having a default config at all
>> for type='lun' devices.
>
> As I said before, that causes apps to have to do special handling
> for iSCSI pools, which is not something we want.
This is not about iSCSI pools, it is about type='lun' devices.
And as I said before, knowing the kind of pool is something you have to
do anyway for type='lun' devices, because they won't work if the
underlying pool is filesystem- or LVM-based.
Regardless of whether all pools types can use type=lun, I don't want
to see special case handling of this attribute. If it is optional in
the XML schema, it should be unconditionally optional for any usage.
>> Also, can libvirt at least detect using a volume on a pool
that wasn't
>> started, and fail unless mode='direct'?
>
> It should always fail to start the guest if the pool is not started,
> regardless of the mode used.
Another point of mode='direct' is that the pool need not be started.
That is not something we can allow in the long term. When we expand our
fine grained access control further, we're likely going to need to do
ACL checks for the virStorageVolPtr object referenced by the XML config
here. We can't do that unless the pool is running. So we cannot set a
precedent of allowing use without the pool being active IMHO.
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 :|
5:59 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 24/07/2013 12:46, Daniel P. Berrange ha scritto:
On Wed, Jul 24, 2013 at 12:41:49PM +0200, Paolo Bonzini wrote:
> Il 24/07/2013 12:39, Daniel P. Berrange ha scritto:
>> On Wed, Jul 24, 2013 at 12:37:10PM +0200, Paolo Bonzini wrote:
>>> Il 24/07/2013 12:31, Daniel P. Berrange ha scritto:
>>>> breaking the default config for QEMU's without iSCSI support.
>>>
>>> Understood, that's why I suggested not having a default config at all
>>> for type='lun' devices.
>>
>> As I said before, that causes apps to have to do special handling
>> for iSCSI pools, which is not something we want.
>
> This is not about iSCSI pools, it is about type='lun' devices.
>
> And as I said before, knowing the kind of pool is something you have to
> do anyway for type='lun' devices, because they won't work if the
> underlying pool is filesystem- or LVM-based.
Regardless of whether all pools types can use type=lun, I don't want
to see special case handling of this attribute. If it is optional in
the XML schema, it should be unconditionally optional for any usage.
The XML schema can mark it as optional for type!=lun and mandatory for
type=lun, but I agree it may be unnecessarily complicated.
>>> Also, can libvirt at least detect using a volume on a
pool that wasn't
>>> started, and fail unless mode='direct'?
>>
>> It should always fail to start the guest if the pool is not started,
>> regardless of the mode used.
>
> Another point of mode='direct' is that the pool need not be started.
That is not something we can allow in the long term. When we expand our
fine grained access control further, we're likely going to need to do
ACL checks for the virStorageVolPtr object referenced by the XML config
here. We can't do that unless the pool is running. So we cannot set a
precedent of allowing use without the pool being active IMHO.
Can you make the pool active, but at the same time not expose it as
devices on the host? LUNs can be discovered by talking directly to the
target (similar to the iscsi-ls command included with libiscsi).
In other words, perhaps mode='host' vs. mode='direct' could be a
property of the pool rather than the disk? That would certainly work
fine for me.
Paolo
6:03 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Wed, Jul 24, 2013 at 12:59:27PM +0200, Paolo Bonzini wrote:
Il 24/07/2013 12:46, Daniel P. Berrange ha scritto:
> On Wed, Jul 24, 2013 at 12:41:49PM +0200, Paolo Bonzini wrote:
>> Il 24/07/2013 12:39, Daniel P. Berrange ha scritto:
>>> On Wed, Jul 24, 2013 at 12:37:10PM +0200, Paolo Bonzini wrote:
>>>> Il 24/07/2013 12:31, Daniel P. Berrange ha scritto:
>>>>> breaking the default config for QEMU's without iSCSI support.
>>>>
>>>> Understood, that's why I suggested not having a default config at
all
>>>> for type='lun' devices.
>>>
>>> As I said before, that causes apps to have to do special handling
>>> for iSCSI pools, which is not something we want.
>>
>> This is not about iSCSI pools, it is about type='lun' devices.
>>
>> And as I said before, knowing the kind of pool is something you have to
>> do anyway for type='lun' devices, because they won't work if the
>> underlying pool is filesystem- or LVM-based.
>
> Regardless of whether all pools types can use type=lun, I don't want
> to see special case handling of this attribute. If it is optional in
> the XML schema, it should be unconditionally optional for any usage.
The XML schema can mark it as optional for type!=lun and mandatory for
type=lun, but I agree it may be unnecessarily complicated.
>>>> Also, can libvirt at least detect using a volume on a pool that
wasn't
>>>> started, and fail unless mode='direct'?
>>>
>>> It should always fail to start the guest if the pool is not started,
>>> regardless of the mode used.
>>
>> Another point of mode='direct' is that the pool need not be started.
>
> That is not something we can allow in the long term. When we expand our
> fine grained access control further, we're likely going to need to do
> ACL checks for the virStorageVolPtr object referenced by the XML config
> here. We can't do that unless the pool is running. So we cannot set a
> precedent of allowing use without the pool being active IMHO.
Can you make the pool active, but at the same time not expose it as
devices on the host? LUNs can be discovered by talking directly to the
target (similar to the iscsi-ls command included with libiscsi).
I've never considered that actually. From the API POV, what we need to
know is the list of LUNS and their sizes. We don't require that they
have files visible in the host - for example the VMWare storage pool
impls don't expose files on the host.
So adding a mode for the pool which let you enumerate LUNs, but not
expose them in the FS would be an option.
In other words, perhaps mode='host' vs. mode='direct'
could be a
property of the pool rather than the disk? That would certainly work
fine for me.
It doesn't neccessarily need to be exclusive. If the pool used
mode='host' we could still allow mode=host|direct in the guest. Only
if the pool used mode=direct, would have to restrict it to mode=direct
in the guest. We could make the guest config setting for "mode"
default to match the pool's config.
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 :|
6:24 a.m.
New subject: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 24/07/2013 13:03, Daniel P. Berrange ha scritto:
> Can you make the pool active, but at the same time not expose it
as
> devices on the host? LUNs can be discovered by talking directly to the
> target (similar to the iscsi-ls command included with libiscsi).
I've never considered that actually. From the API POV, what we need to
know is the list of LUNS and their sizes. We don't require that they
have files visible in the host - for example the VMWare storage pool
impls don't expose files on the host.
So adding a mode for the pool which let you enumerate LUNs, but not
expose them in the FS would be an option.
> In other words, perhaps mode='host' vs. mode='direct' could be a
> property of the pool rather than the disk? That would certainly work
> fine for me.
It doesn't neccessarily need to be exclusive. If the pool used
mode='host' we could still allow mode=host|direct in the guest. Only
if the pool used mode=direct, would have to restrict it to mode=direct
in the guest. We could make the guest config setting for "mode"
default to match the pool's config.
Cool! Sounds like a plan.
libvirt itself could use libiscsi to support discovery for mode='direct'
pools.
Paolo
7:32 a.m.
New subject: [libvirt] [PATCH v3 5/7] tests: Add various network and volume definitions
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..bd5d755
--- /dev/null
+++ b/tests/securityselinuxlabeldata/voldisks.txt
@@ -0,0 +1,5 @@
+/file.raw;
+/disk.raw;
+/host.raw;
+/direct.raw;
+/cdrom.raw;
diff --git a/tests/securityselinuxlabeldata/voldisks.xml
b/tests/securityselinuxlabeldata/voldisks.xml
new file mode 100644
index 0000000..ae7e629
--- /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='direct' file='/direct.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
6:06 a.m.
New subject: [libvirt] [PATCH v3 5/7] tests: Add various network and volume definitions
On 19/07/13 20:32, John Ferlan wrote:
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">
i'm not clear with the security tests, but this xml looks incorrect.
"file" is one
way to represent the disk source, it's exclusive with other ways (e.g.
protocol/name
here) in semantics. similar for below. why do you use both "file" and
other ways
for disk source represention together?
+ <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..bd5d755
--- /dev/null
+++ b/tests/securityselinuxlabeldata/voldisks.txt
@@ -0,0 +1,5 @@
+/file.raw;
+/disk.raw;
+/host.raw;
+/direct.raw;
+/cdrom.raw;
diff --git a/tests/securityselinuxlabeldata/voldisks.xml
b/tests/securityselinuxlabeldata/voldisks.xml
new file mode 100644
index 0000000..ae7e629
--- /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='direct' file='/direct.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");
10:40 a.m.
New subject: [libvirt] [PATCH v3 5/7] tests: Add various network and volume definitions
On 07/22/2013 07:06 AM, Osier Yang wrote:
On 19/07/13 20:32, John Ferlan wrote:
> 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">
i'm not clear with the security tests, but this xml looks incorrect.
"file" is one
way to represent the disk source, it's exclusive with other ways (e.g.
protocol/name
here) in semantics. similar for below. why do you use both "file" and
other ways
for disk source represention together?
Following syntax found in the following files
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-*.xml
where '*' is {gluster, nbd, rbd, sheepdog, & iscsi}
I can hold off pushing this patch if desired.
The 'file' names are found in the 'netdisks.txt' file which are where
the seclabels get listed for other tests. For the network types there
are no seclabels.
John
> + <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..bd5d755
> --- /dev/null
> +++ b/tests/securityselinuxlabeldata/voldisks.txt
> @@ -0,0 +1,5 @@
> +/file.raw;
> +/disk.raw;
> +/host.raw;
> +/direct.raw;
> +/cdrom.raw;
> diff --git a/tests/securityselinuxlabeldata/voldisks.xml
> b/tests/securityselinuxlabeldata/voldisks.xml
> new file mode 100644
> index 0000000..ae7e629
> --- /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='direct'
> file='/direct.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");
>
10:24 p.m.
New subject: [libvirt] [PATCH v3 5/7] tests: Add various network and volume definitions
On 22/07/13 23:40, John Ferlan wrote:
On 07/22/2013 07:06 AM, Osier Yang wrote:
> On 19/07/13 20:32, John Ferlan wrote:
>> 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">
> i'm not clear with the security tests, but this xml looks incorrect.
> "file" is one
> way to represent the disk source, it's exclusive with other ways (e.g.
> protocol/name
> here) in semantics. similar for below. why do you use both "file" and
> other ways
> for disk source represention together?
>
Following syntax found in the following files
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-*.xml
where '*' is {gluster, nbd, rbd, sheepdog, & iscsi}
I can hold off pushing this patch if desired.
i believe you will need a further patch to fix it if it's pushed. :\
The 'file' names are found in the 'netdisks.txt' file
which are where
the seclabels get listed for other tests. For the network types there
are no seclabels.
these still don't answer my question, the (file="$file") and other
representation
ways are conflicted in semantics, though xml parsing could report no error,
since one of the ways (e.g. "file") is ignored when parsing.
see the disk source rng schema:
<group>
<attribute name="type">
<value>file</value>
</attribute>
<interleave>
<optional>
<element name="source">
<optional>
<attribute name="file">
<ref name="absFilePath"/>
</attribute>
</optional>
<optional>
<ref name="startupPolicy"/>
</optional>
<optional>
<ref name='devSeclabel'/>
</optional>
</element>
</optional>
<ref name="diskspec"/>
</interleave>
</group>
<group>
<attribute name="type">
<value>block</value>
</attribute>
<interleave>
<optional>
<element name="source">
<attribute name="dev">
<ref name="absFilePath"/>
</attribute>
<optional>
<ref name='devSeclabel'/>
</optional>
</element>
</optional>
<ref name="diskspec"/>
</interleave>
</group>
<group>
<attribute name="type">
<value>dir</value>
</attribute>
<interleave>
<optional>
<element name="source">
<attribute name="dir">
<ref name="absFilePath"/>
</attribute>
<empty/>
</element>
</optional>
<ref name="diskspec"/>
</interleave>
</group>
<group>
<attribute name="type">
<value>network</value>
</attribute>
<interleave>
<optional>
<element name="source">
<ref name='diskSourceNetwork'/>
</element>
</optional>
<ref name="diskspec"/>
</interleave>
</group>
<group>
<attribute name="type">
<value>volume</value>
</attribute>
<interleave>
<optional>
<element name="source">
<attribute name="pool">
<ref name="genericName"/>
</attribute>
<attribute name="volume">
<ref name="volName"/>
</attribute>
<optional>
<ref name="startupPolicy"/>
</optional>
<optional>
<ref name='devSeclabel'/>
</optional>
</element>
</optional>
<ref name="diskspec"/>
</interleave>
</group>
this patch is not that important for the feature. so i think it can be
delayed till
there is a right solution.
John
>> + <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..bd5d755
>> --- /dev/null
>> +++ b/tests/securityselinuxlabeldata/voldisks.txt
>> @@ -0,0 +1,5 @@
>> +/file.raw;
>> +/disk.raw;
>> +/host.raw;
>> +/direct.raw;
>> +/cdrom.raw;
>> diff --git a/tests/securityselinuxlabeldata/voldisks.xml
>> b/tests/securityselinuxlabeldata/voldisks.xml
>> new file mode 100644
>> index 0000000..ae7e629
>> --- /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='direct'
>> file='/direct.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");
>>
7:32 a.m.
New subject: [libvirt] [PATCH v3 6/7] conf: Ignore the volume type disk if its mode is "direct"
From: Osier Yang <jyang(a)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="direct" 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 231edb5..770b7cc 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_DIRECT))
return 0;
if (iter(disk, disk->src, 0, opaque) < 0)
--
1.8.1.4
6:07 a.m.
New subject: [libvirt] [PATCH v3 6/7] conf: Ignore the volume type disk if its mode is "direct"
On 19/07/13 20:32, John Ferlan wrote:
From: Osier Yang <jyang(a)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="direct" 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 231edb5..770b7cc 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_DIRECT))
return 0;
if (iter(disk, disk->src, 0, opaque) < 0)
ack
7:32 a.m.
New subject: [libvirt] [PATCH v3 7/7] qemu: Translate the volume type disk source before cgroup setting
From: Osier Yang <jyang(a)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 ef81536..d7deb0f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3627,6 +3627,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;
@@ -3671,11 +3679,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
6:09 a.m.
New subject: [libvirt] [PATCH v3 7/7] qemu: Translate the volume type disk source before cgroup setting
On 19/07/13 20:32, John Ferlan wrote:
From: Osier Yang <jyang(a)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 ef81536..d7deb0f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3627,6 +3627,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;
@@ -3671,11 +3679,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,
ack, a
bit strange to ack my own patch though. may need to wait for a
second ack from others.
1:59 p.m.
On 07/19/2013 08:32 AM, John Ferlan wrote:
An update to yesterday's posting:
https://www.redhat.com/archives/libvir-list/2013-July/msg01213.html
Changes in v3 over v2
* Update patch 1 per code review
* Use 'direct' instead of 'uri' and supporting documentaiton
* In patch 4, use qemuAddISCSIPoolSourceHost() instead of
qemuAddDiskISCSIUri. Also added a check in the function
that (pooldef->source.nhost != 1) to make sure we at least
have a host defined in the pool and of course that there's
only one.
John Ferlan (5):
storage_iscsi: Reflect the default target port
conf: Introduce new XML tag "mode" for disk source
conf: Introduce virDomainDiskSourceIsBlockType
qemu: Translate the iscsi pool/volume disk source
tests: Add various network and volume definitions
Osier Yang (2):
conf: Ignore the volume type disk if its mode is "direct"
qemu: Translate the volume type disk source before cgroup setting
docs/formatdomain.html.in | 11 +-
docs/schemas/domaincommon.rng | 8 ++
src/conf/domain_conf.c | 67 ++++++++++-
src/conf/domain_conf.h | 26 +++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 20 +++-
src/qemu/qemu_conf.c | 124 +++++++++++++++++----
src/qemu/qemu_process.c | 13 ++-
src/storage/storage_backend_iscsi.c | 19 ++--
.../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, 414 insertions(+), 39 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
I pushed patches 1-4, 6, & 7. Will wait for more feedback on 5.
Thanks for the reviews.
John
4142
days inactive
4147
days old
41 comments
4 participants
participants (4)
-
Daniel P. Berrange -
John Ferlan -
Osier Yang -
Paolo Bonzini