[libvirt] [PATCH 0/2] Add multiIQN XML dump output
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
--- 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
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 :|
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
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 :|
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 :|
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 :|
* 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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (6)
-
Daniel P. Berrange -
Dave Allan -
David Allan
-
Eric Blake -
Hugh O. Brock -
Justin Clift