[libvirt] [PATCH] storage: Do not use comma as seperator for lvs output
* src/storage/storage_backend_logical.c: If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here): test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304 The RE we uses: const char *regexes[] = { "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" }; This patch changes the seperator into "#" to fix the problem. Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..6c13a6b 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -199,7 +199,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL -- 1.7.6
On Wed, Sep 21, 2011 at 01:39:22PM +0800, Osier Yang wrote:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we uses:
const char *regexes[] = { "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" };
This patch changes the seperator into "#" to fix the problem.
Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..6c13a6b 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -199,7 +199,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL
Err, it seems the regexp should be changed too ... as you wrote it seems to expect ',' as the separator. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
于 2011年09月21日 14:41, Daniel Veillard 写道:
On Wed, Sep 21, 2011 at 01:39:22PM +0800, Osier Yang wrote:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we uses:
const char *regexes[] = { "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" };
This patch changes the seperator into "#" to fix the problem.
Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..6c13a6b 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -199,7 +199,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL Err, it seems the regexp should be changed too ... as you wrote it seems to expect ',' as the separator.
Oops, updated patch is following. Osier
* src/storage/storage_backend_logical.c: If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here): test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304 The RE we uses: const char *regexes[] = { "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" }; This patch changes the seperator into "#" to fix the problem. Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..45f77ad 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -187,19 +187,20 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, * * NB can be multiple rows per volume if they have many extents * - * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line + * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing + * @separator on each line * * NB Encrypted logical volumes can print ':' in their name, so it is * not a suitable separator (rhbz 470693). */ const char *regexes[] = { - "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" + "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)\\((\\S+)\\)#(\\S+)#([0-9]+)#?\\s*$" }; int vars[] = { 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL -- 1.7.6
On Wed, Sep 21, 2011 at 05:07:56PM +0800, Osier Yang wrote:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
If the 'lv' output contains multiple paths tin the "device" field, then the issue of ',' vs '#' is the least of our worries. We use the path in the device field to populate the volume extents information. if ((vol->source.extents[vol->source.nextent].path = strdup(groups[3])) == NULL) { virReportOOMError(); return -1; } ... vol->source.extents[vol->source.nextent].start = offset * size; vol->source.extents[vol->source.nextent].end = (offset * size) + length; vol->source.nextent++; This code will need significantly rewriting to cope with multiple device paths when parsing the output, before we can make the change you suggest here. Regards, 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 :|
于 2011年09月21日 18:22, Daniel P. Berrange 写道:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304 If the 'lv' output contains multiple paths tin the "device" field,
On Wed, Sep 21, 2011 at 05:07:56PM +0800, Osier Yang wrote: then the issue of ',' vs '#' is the least of our worries.
We use the path in the device field to populate the volume extents information.
if ((vol->source.extents[vol->source.nextent].path = strdup(groups[3])) == NULL) { virReportOOMError(); return -1; } ...
vol->source.extents[vol->source.nextent].start = offset * size; vol->source.extents[vol->source.nextent].end = (offset * size) + length; vol->source.nextent++;
This code will need significantly rewriting to cope with multiple device paths when parsing the output, before we can make the change you suggest here.
Regards, Daniel
Hmm, agreed, will make a new patch to cover this. Thanks Osier
hi Osier : 于 2011年09月21日 17:07, Osier Yang 写道:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we uses:
const char *regexes[] = { "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" };
This patch changes the seperator into "#" to fix the problem.
Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..45f77ad 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -187,19 +187,20 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, * * NB can be multiple rows per volume if they have many extents * - * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line + * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing + * @separator on each line * * NB Encrypted logical volumes can print ':' in their name, so it is * not a suitable separator (rhbz 470693). */ const char *regexes[] = { - "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" + "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)\\((\\S+)\\)#(\\S+)#([0-9]+)#?\\s*$" }; int vars[] = { 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL
I reproduced the bug : [root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd error: Failed to create pool vg_ssd error: cannot open volume '/dev/vg_ssd/test_stripes,': No such file or directory and then I tested this patch , it seems work well. [root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd Pool vg_ssd created [root@localhost bin]# ./virsh pool-info vg_ssd Name: vg_ssd UUID: c45cc84e-7879-cc15-ee78-2d2dda6b531d State: running Persistent: no Autostart: no Capacity: 200.00 MB Allocation: 152.00 MB Available: 48.00 MB
于 2011年09月22日 14:31, Eli 写道:
hi Osier : 于 2011年09月21日 17:07, Osier Yang 写道:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we uses:
const char *regexes[] = {
"^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" };
This patch changes the seperator into "#" to fix the problem.
Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..45f77ad 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -187,19 +187,20 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, * * NB can be multiple rows per volume if they have many extents * - * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line + * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing + * @separator on each line * * NB Encrypted logical volumes can print ':' in their name, so it is * not a suitable separator (rhbz 470693). */ const char *regexes[] = { - "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" + "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)\\((\\S+)\\)#(\\S+)#([0-9]+)#?\\s*$" }; int vars[] = { 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL
I reproduced the bug :
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd error: Failed to create pool vg_ssd error: cannot open volume '/dev/vg_ssd/test_stripes,': No such file or directory
and then I tested this patch , it seems work well.
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd Pool vg_ssd created
[root@localhost bin]# ./virsh pool-info vg_ssd Name: vg_ssd UUID: c45cc84e-7879-cc15-ee78-2d2dda6b531d State: running Persistent: no Autostart: no Capacity: 200.00 MB Allocation: 152.00 MB Available: 48.00 MB
Thanks for the testing, Eli, Could you also check what's the vol XML? I want to confirm if the "<extents>" in "<source><device></device></source>"displays well, though it looks to me there is no "<path>" element defined for "<extents>" in the storage vol schema yet. Regards, Osier
hi Osier: 于 2011年09月22日 15:08, Osier Yang 写道:
于 2011年09月22日 14:31, Eli 写道:
hi Osier : 于 2011年09月21日 17:07, Osier Yang 写道:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we uses:
const char *regexes[] = {
"^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" };
This patch changes the seperator into "#" to fix the problem.
Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..45f77ad 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -187,19 +187,20 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, * * NB can be multiple rows per volume if they have many extents * - * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line + * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing + * @separator on each line * * NB Encrypted logical volumes can print ':' in their name, so it is * not a suitable separator (rhbz 470693). */ const char *regexes[] = { - "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" + "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)\\((\\S+)\\)#(\\S+)#([0-9]+)#?\\s*$" }; int vars[] = { 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL
I reproduced the bug :
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd error: Failed to create pool vg_ssd error: cannot open volume '/dev/vg_ssd/test_stripes,': No such file or directory
and then I tested this patch , it seems work well.
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd Pool vg_ssd created
[root@localhost bin]# ./virsh pool-info vg_ssd Name: vg_ssd UUID: c45cc84e-7879-cc15-ee78-2d2dda6b531d State: running Persistent: no Autostart: no Capacity: 200.00 MB Allocation: 152.00 MB Available: 48.00 MB
Thanks for the testing, Eli,
Could you also check what's the vol XML? I want to confirm if the "<extents>" in "<source><device></device></source>"displays well, though it looks to me there is no "<path>" element defined for "<extents>" in the storage vol schema yet.
1. my vol XML is like this: virsh # vol-dumpxml test_stripes --pool vg_ssd <volume> <name>test_stripes</name> <key>1pc6Gf-1hn2-WGnw-ASKt-uX6w-xUed-qERq50</key> *<source> <device path='/dev/sdb(0),/dev/sdc'> <extent start='0' end='159383552'/> </device> </source>* <capacity>159383552</capacity> <allocation>159383552</allocation> <target> <path>/dev/vg_ssd/test_stripes</path> <permissions> <mode>0660</mode> <owner>0</owner> <group>6</group> <label>system_u:object_r:fixed_disk_device_t:s0</label> </permissions> </target> </volume> 2. look at the docs/schemas/storagevol.rng you can see that: <define name='source'> <element name='source'> <zeroOrMore> <ref name='sourcedev'/> </zeroOrMore> </element> </define> *<define name='sourcedev'>* <element name='device'> <attribute name='path'> <ref name='path'/> </attribute> <choice> <empty/> *<ref name='devextents'/>* </choice> </element> </define> *<define name='devextents'>* <oneOrMore> <element name='extent'> <attribute name='start'> <ref name='uint'/> </attribute> <attribute name='end'> <ref name='uint'/> </attribute> </element> </oneOrMore> </define> ////////////////// so the xml should like this <source> <device path=xxx> <extent start= end=/> </device> </source> ps: the xml just shows the 'element name' not 'define name' that defined in the schema file.
Regards, Osier best regards eli
于 2011年09月22日 16:41, Eli 写道:
hi Osier: 于 2011年09月22日 15:08, Osier Yang 写道:
于 2011年09月22日 14:31, Eli 写道:
hi Osier : 于 2011年09月21日 17:07, Osier Yang 写道:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we uses:
const char *regexes[] = {
"^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" };
This patch changes the seperator into "#" to fix the problem.
Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..45f77ad 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -187,19 +187,20 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, * * NB can be multiple rows per volume if they have many extents * - * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line + * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing + * @separator on each line * * NB Encrypted logical volumes can print ':' in their name, so it is * not a suitable separator (rhbz 470693). */ const char *regexes[] = { - "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" + "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)\\((\\S+)\\)#(\\S+)#([0-9]+)#?\\s*$" }; int vars[] = { 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL
I reproduced the bug :
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd error: Failed to create pool vg_ssd error: cannot open volume '/dev/vg_ssd/test_stripes,': No such file or directory
and then I tested this patch , it seems work well.
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd Pool vg_ssd created
[root@localhost bin]# ./virsh pool-info vg_ssd Name: vg_ssd UUID: c45cc84e-7879-cc15-ee78-2d2dda6b531d State: running Persistent: no Autostart: no Capacity: 200.00 MB Allocation: 152.00 MB Available: 48.00 MB
Thanks for the testing, Eli,
Could you also check what's the vol XML? I want to confirm if the "<extents>" in "<source><device></device></source>"displays well, though it looks to me there is no "<path>" element defined for "<extents>" in the storage vol schema yet.
1. my vol XML is like this:
virsh # vol-dumpxml test_stripes --pool vg_ssd <volume> <name>test_stripes</name> <key>1pc6Gf-1hn2-WGnw-ASKt-uX6w-xUed-qERq50</key> *<source> <device path='/dev/sdb(0),/dev/sdc'> *
This is expected, and is what's Daniel Berrange worried about.
*<extent start='0' end='159383552'/> </device> </source>*
Thanks Osier
于 2011年09月22日 17:08, Osier Yang 写道:
于 2011年09月22日 16:41, Eli 写道:
hi Osier: 于 2011年09月22日 15:08, Osier Yang 写道:
于 2011年09月22日 14:31, Eli 写道:
hi Osier : 于 2011年09月21日 17:07, Osier Yang 写道:
* src/storage/storage_backend_logical.c:
If a logical vol is created with multiple stripes. (e.g. --stripes 3), the "device" field of lvs output will have multiple fileds which are seperated by comma. It means the RE we write in the codes will not work well anymore. E.g. (lvs output for a stripped vol, uses "#" as seperator here):
test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\ /dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304
The RE we uses:
const char *regexes[] = {
"^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" };
This patch changes the seperator into "#" to fix the problem.
Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474 --- src/storage/storage_backend_logical.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4f42047..45f77ad 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -187,19 +187,20 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, * * NB can be multiple rows per volume if they have many extents * - * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line + * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing + * @separator on each line * * NB Encrypted logical volumes can print ':' in their name, so it is * not a suitable separator (rhbz 470693). */ const char *regexes[] = { - "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$" + "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)\\((\\S+)\\)#(\\S+)#([0-9]+)#?\\s*$" }; int vars[] = { 7 }; const char *prog[] = { - LVS, "--separator", ",", "--noheadings", "--units", "b", + LVS, "--separator", "#", "--noheadings", "--units", "b", "--unbuffered", "--nosuffix", "--options", "lv_name,origin,uuid,devices,seg_size,vg_extent_size", pool->def->source.name, NULL
I reproduced the bug :
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd error: Failed to create pool vg_ssd error: cannot open volume '/dev/vg_ssd/test_stripes,': No such file or directory
and then I tested this patch , it seems work well.
[root@localhost bin]# ./virsh -d 5 pool-create-as vg_ssd logical --target /dev/vg_ssd Pool vg_ssd created
[root@localhost bin]# ./virsh pool-info vg_ssd Name: vg_ssd UUID: c45cc84e-7879-cc15-ee78-2d2dda6b531d State: running Persistent: no Autostart: no Capacity: 200.00 MB Allocation: 152.00 MB Available: 48.00 MB
Thanks for the testing, Eli,
Could you also check what's the vol XML? I want to confirm if the "<extents>" in "<source><device></device></source>"displays well, though it looks to me there is no "<path>" element defined for "<extents>" in the storage vol schema yet.
1. my vol XML is like this:
virsh # vol-dumpxml test_stripes --pool vg_ssd <volume> <name>test_stripes</name> <key>1pc6Gf-1hn2-WGnw-ASKt-uX6w-xUed-qERq50</key> *<source> <device path='/dev/sdb(0),/dev/sdc'> *
do you mean the xml should like this : <device path='/dev/sdb(0)'> <extent start='0' end='xxx'/> </device> <device path='/dev/sdc'> <extent start='xxx' end='yyy'/> </device>
This is expected, and is what's Daniel Berrange worried about.
*<extent start='0' end='159383552'/> </device> </source>*
Thanks Osier
-- best regards eli
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Eli -
Eli Qiao
-
Osier Yang