3:58 p.m.
The first of these two patches adds XML output for multi-IQN iSCSI pools. The second
makes the rng consistent with the existing code and adds regression tests. My RelaxNG
skills are rusty, so I would much appreciate a careful review of the rng change.
Dave
David Allan (2):
Add multiiqn XML dump
Add multiIQN tests
docs/schemas/storagepool.rng | 16 ++++++++------
src/conf/storage_conf.c | 7 ++++++
tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml | 22 ++++++++++++++++++++
.../storagepoolxml2xmlout/pool-iscsi-multiiqn.xml | 22 ++++++++++++++++++++
tests/storagepoolxml2xmltest.c | 1 +
5 files changed, 61 insertions(+), 7 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml
3:58 p.m.
New subject: [libvirt] [PATCH 1/2] Add multiiqn XML dump
---
src/conf/storage_conf.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 422e76a..7ed49a1 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -815,6 +815,13 @@ virStoragePoolSourceFormat(virBufferPtr buf,
src->name)
virBufferVSprintf(buf," <name>%s</name>\n",
src->name);
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) &&
+ src->initiator.iqn) {
+ virBufferAddLit(buf," <initiator>\n");
+ virBufferVSprintf(buf," <iqn name='%s'/>\n",
src->initiator.iqn);
+ virBufferAddLit(buf," </initiator>\n");
+ }
+
if (options->formatToString) {
const char *format = (options->formatToString)(src->format);
if (!format) {
--
1.7.0.1
3:40 a.m.
New subject: [libvirt] [PATCH 1/2] Add multiiqn XML dump
On Mon, Jun 07, 2010 at 04:58:08PM -0400, David Allan wrote:
---
src/conf/storage_conf.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 422e76a..7ed49a1 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -815,6 +815,13 @@ virStoragePoolSourceFormat(virBufferPtr buf,
src->name)
virBufferVSprintf(buf," <name>%s</name>\n",
src->name);
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) &&
+ src->initiator.iqn) {
+ virBufferAddLit(buf," <initiator>\n");
+ virBufferVSprintf(buf," <iqn name='%s'/>\n",
src->initiator.iqn);
For paranoia, we should probably use virBufferEscapeString() in this place
+ virBufferAddLit(buf," </initiator>\n");
+ }
+
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5:20 a.m.
New subject: [libvirt] [PATCH 1/2] Add multiiqn XML dump
On 06/08/2010 06:40 PM, Daniel P. Berrange wrote:
<snip>
For paranoia, we should probably use virBufferEscapeString() in this
place
Found virBufferEscapeString() in buf.c/h, but it's not listed on the API
Reference page on the website.
Is it supposed to be?
Regards and best wishes,
Justin Clift
--
Salasaga - Open Source eLearning IDE
http://www.salasaga.org
5:31 a.m.
New subject: [libvirt] [PATCH 1/2] Add multiiqn XML dump
On Tue, Jun 08, 2010 at 08:20:37PM +1000, Justin Clift wrote:
On 06/08/2010 06:40 PM, Daniel P. Berrange wrote:
<snip>
>For paranoia, we should probably use virBufferEscapeString() in this place
Found virBufferEscapeString() in buf.c/h, but it's not listed on the API
Reference page on the website.
Is it supposed to be?
This is an internal use only API. Only things listed in include/libvirt
are supposed to be documented in the API reference
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:04 a.m.
New subject: [libvirt] [PATCH 1/2] Add multiiqn XML dump
On Tue, Jun 08, 2010 at 11:31:21AM +0100, Daniel P. Berrange wrote:
On Tue, Jun 08, 2010 at 08:20:37PM +1000, Justin Clift wrote:
> On 06/08/2010 06:40 PM, Daniel P. Berrange wrote:
> <snip>
> >For paranoia, we should probably use virBufferEscapeString() in this place
>
> Found virBufferEscapeString() in buf.c/h, but it's not listed on the API
> Reference page on the website.
>
> Is it supposed to be?
This is an internal use only API. Only things listed in include/libvirt
are supposed to be documented in the API reference
Such things should have a place, though -- maybe the HACKING file?
--H
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:12 a.m.
New subject: [libvirt] [PATCH 1/2] Add multiiqn XML dump
On Tue, Jun 08, 2010 at 10:04:56AM -0400, Hugh O. Brock wrote:
On Tue, Jun 08, 2010 at 11:31:21AM +0100, Daniel P. Berrange wrote:
> On Tue, Jun 08, 2010 at 08:20:37PM +1000, Justin Clift wrote:
> > On 06/08/2010 06:40 PM, Daniel P. Berrange wrote:
> > <snip>
> > >For paranoia, we should probably use virBufferEscapeString() in this place
> >
> > Found virBufferEscapeString() in buf.c/h, but it's not listed on the API
> > Reference page on the website.
> >
> > Is it supposed to be?
>
> This is an internal use only API. Only things listed in include/libvirt
> are supposed to be documented in the API reference
Such things should have a place, though -- maybe the HACKING file?
Yes, but the HACKING file really needs to be split up into separate
pages under the Docs -> Internals section of the website. One page
couldbe the virBuffer APIs
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
1:28 p.m.
New subject: [libvirt] [PATCH 1/2] Add multiiqn XML dump
On Tue, Jun 08, 2010 at 09:40:30AM +0100, Daniel P. Berrange wrote:
On Mon, Jun 07, 2010 at 04:58:08PM -0400, David Allan wrote:
> ---
> src/conf/storage_conf.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 422e76a..7ed49a1 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -815,6 +815,13 @@ virStoragePoolSourceFormat(virBufferPtr buf,
> src->name)
> virBufferVSprintf(buf," <name>%s</name>\n",
src->name);
>
> + if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) &&
> + src->initiator.iqn) {
> + virBufferAddLit(buf," <initiator>\n");
> + virBufferVSprintf(buf," <iqn name='%s'/>\n",
src->initiator.iqn);
For paranoia, we should probably use virBufferEscapeString() in this place
Made that change and pushed. Thanks.
Dave
> + virBufferAddLit(buf,"
</initiator>\n");
> + }
> +
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
3:58 p.m.
New subject: [libvirt] [PATCH 2/2] Add multiIQN tests
* Fix broken rng schema
* Add test input & output files
---
docs/schemas/storagepool.rng | 16 ++++++++------
tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml | 22 ++++++++++++++++++++
.../storagepoolxml2xmlout/pool-iscsi-multiiqn.xml | 22 ++++++++++++++++++++
tests/storagepoolxml2xmltest.c | 1 +
4 files changed, 54 insertions(+), 7 deletions(-)
create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index cfcf9a6..b911f7c 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -188,12 +188,14 @@
</element>
</define>
- <define name='initiatorinfoiqn'>
- <element name='iqn'>
- <attribute name='name'>
- <text/>
- </attribute>
- <empty/>
+ <define name='initiatorinfo'>
+ <element name='initiator'>
+ <element name='iqn'>
+ <attribute name='name'>
+ <text/>
+ </attribute>
+ <empty/>
+ </element>
</element>
</define>
@@ -372,7 +374,7 @@
<ref name='sourceinfohost'/>
<ref name='sourceinfodev'/>
<optional>
- <ref name='initiatorinfoiqn'/>
+ <ref name='initiatorinfo'/>
</optional>
<optional>
<ref name='sourceinfoauth'/>
diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml
b/tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml
new file mode 100644
index 0000000..4c77086
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml
@@ -0,0 +1,22 @@
+<pool type='iscsi'>
+ <name>multiiqn</name>
+ <uuid>e9392370-2917-565e-792c-e057f46512d7</uuid>
+ <capacity>0</capacity>
+ <allocation>0</allocation>
+ <available>0</available>
+ <source>
+ <host name='iscsi.example.com'/>
+ <device path='demo-target'/>
+ <initiator>
+ <iqn name='initiator0'/>
+ </initiator>
+ </source>
+ <target>
+ <path>/dev/disk/by-path</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml
b/tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml
new file mode 100644
index 0000000..4c77086
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-iscsi-multiiqn.xml
@@ -0,0 +1,22 @@
+<pool type='iscsi'>
+ <name>multiiqn</name>
+ <uuid>e9392370-2917-565e-792c-e057f46512d7</uuid>
+ <capacity>0</capacity>
+ <allocation>0</allocation>
+ <available>0</available>
+ <source>
+ <host name='iscsi.example.com'/>
+ <device path='demo-target'/>
+ <initiator>
+ <iqn name='initiator0'/>
+ </initiator>
+ </source>
+ <target>
+ <path>/dev/disk/by-path</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 4550407..33a7343 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -95,6 +95,7 @@ mymain(int argc, char **argv)
DO_TEST("pool-netfs");
DO_TEST("pool-scsi");
DO_TEST("pool-mpath");
+ DO_TEST("pool-iscsi-multiiqn");
return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
--
1.7.0.1
3:12 p.m.
New subject: [libvirt] [PATCH 2/2] Add multiIQN tests
On 06/07/2010 02:58 PM, David Allan wrote:
* Fix broken rng schema
* Add test input & output files
---
docs/schemas/storagepool.rng | 16 ++++++++------
tests/storagepoolxml2xmlin/pool-iscsi-multiiqn.xml | 22 ++++++++++++++++++++
.../storagepoolxml2xmlout/pool-iscsi-multiiqn.xml | 22 ++++++++++++++++++++
tests/storagepoolxml2xmltest.c | 1 +
4 files changed, 54 insertions(+), 7 deletions(-)
And no mention of iqn anywhere in docs/formatstorage.html.in :(
At any rate, I see you already pushed this, but ACK on it looking
correct from my (limited) rng experience.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5281
days inactive
5283
days old
9 comments
6 participants
participants (6)
-
Daniel P. Berrange -
Dave Allan -
David Allan
-
Eric Blake -
Hugh O. Brock -
Justin Clift