[libvirt] [PATCH]Fix minor issues in logical storage backend

Hi All! I came across some problems trying to create a new LVM-based storage pool using this config <pool type="logical"> <name>test_vg</name> <source> <device path="/dev/sdb1"/> </source> <target> <path>/dev/test_vg</path> </target> </pool> Volume group did not previously exist so I did virsh pool-define <above.xml> virsh pool-build test_vg pool-build failed since the backend logical storage driver does not have VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE set in flags. Without this flag set, the device element is never parsed in virStoragePoolDefParseDoc() (storage_conf.c), causing pvcreate to fail since no physical volume is specified. After this problem was memory corruption cause by miscalculating the size of vgcreate command line :-). This patch fixes both issues. Regards, Jim

On Thu, Aug 28, 2008 at 06:15:27PM -0600, Jim Fehlig wrote:
Hi All!
I came across some problems trying to create a new LVM-based storage pool using this config
<pool type="logical"> <name>test_vg</name> <source> <device path="/dev/sdb1"/> </source> <target> <path>/dev/test_vg</path> </target> </pool>
Volume group did not previously exist so I did virsh pool-define <above.xml> virsh pool-build test_vg
pool-build failed since the backend logical storage driver does not have VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE set in flags. Without this flag set, the device element is never parsed in virStoragePoolDefParseDoc() (storage_conf.c), causing pvcreate to fail since no physical volume is specified.
Yep, mea-culpa. I went through so many re-designs & re-writes of the LVM modelling I guess I forgot to test pool creation in the final code :-(
After this problem was memory corruption cause by miscalculating the size of vgcreate command line :-).
Opps, not so much mis-calculating, as not calculating at all !. ACK to this patch Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Aug 28, 2008 at 06:15:27PM -0600, Jim Fehlig wrote:
Hi All!
I came across some problems trying to create a new LVM-based storage pool using this config
<pool type="logical"> <name>test_vg</name> <source> <device path="/dev/sdb1"/> </source> <target> <path>/dev/test_vg</path> </target> </pool>
Volume group did not previously exist so I did virsh pool-define <above.xml> virsh pool-build test_vg
pool-build failed since the backend logical storage driver does not have VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE set in flags. Without this flag set, the device element is never parsed in virStoragePoolDefParseDoc() (storage_conf.c), causing pvcreate to fail since no physical volume is specified.
Hum, looking at the current code now I see .poolOptions = { .flags = VIR_STORAGE_BACKEND_POOL_SOURCE_NAME, .formatFromString = virStorageBackendLogicalPoolFormatFromString, So there is an initialization of the field. It's unclear to me if the two should be OR'ed, or should be kept as-is, if we have the pool name do we need to provide the pool device then ? I get a bit confused now.
After this problem was memory corruption cause by miscalculating the size of vgcreate command line :-).
Whoops !! :-) That one applied cleanly. So at least I'm commiting this part. thanks ! 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/

On Thu, Sep 04, 2008 at 03:13:45PM +0200, Daniel Veillard wrote:
On Thu, Aug 28, 2008 at 06:15:27PM -0600, Jim Fehlig wrote:
Hi All!
I came across some problems trying to create a new LVM-based storage pool using this config
<pool type="logical"> <name>test_vg</name> <source> <device path="/dev/sdb1"/> </source> <target> <path>/dev/test_vg</path> </target> </pool>
Volume group did not previously exist so I did virsh pool-define <above.xml> virsh pool-build test_vg
pool-build failed since the backend logical storage driver does not have VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE set in flags. Without this flag set, the device element is never parsed in virStoragePoolDefParseDoc() (storage_conf.c), causing pvcreate to fail since no physical volume is specified.
Hum, looking at the current code now I see
.poolOptions = { .flags = VIR_STORAGE_BACKEND_POOL_SOURCE_NAME, .formatFromString = virStorageBackendLogicalPoolFormatFromString,
So there is an initialization of the field. It's unclear to me if the two should be OR'ed, or should be kept as-is, if we have the pool name do we need to provide the pool device then ? I get a bit confused now.
The .flag field is a bit-mask, so yes it should be OR'd. Basically the flags are saying which XML elements are valid within the <source> tag, and in LVM case we need to allow both <name> and <device> tags, so they have to be OR'd Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Sep 04, 2008 at 02:21:34PM +0100, Daniel P. Berrange wrote:
On Thu, Sep 04, 2008 at 03:13:45PM +0200, Daniel Veillard wrote:
So there is an initialization of the field. It's unclear to me if the two should be OR'ed, or should be kept as-is, if we have the pool name do we need to provide the pool device then ? I get a bit confused now.
The .flag field is a bit-mask, so yes it should be OR'd. Basically the flags are saying which XML elements are valid within the <source> tag, and in LVM case we need to allow both <name> and <device> tags, so they have to be OR'd
Okay, I commited the OR'ed initialization of the field. 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/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Jim Fehlig