On Wed, Nov 13, 2013 at 12:07:43PM +0100, Peter Krempa wrote:
> Consider the following valid snapshot XML as the <driver> element is
> allowed to be empty in the domainsnapshot.rng schema:
>
> $ cat snap.xml
> <domainsnapshot>
> <disks>
> <disk name='vda' snapshot='external'>
> <source file='/tmp/foo'/>
> <driver/>
> </disk>
> </disks>
> </domainsnapshot>
>
> produces the following error:
>
> $ virsh snapshot-create domain snap.xml
> error: internal error: unknown disk snapshot driver '(null)'
>
> The driver type is parsed as NULL from the XML as the attribute is not
> present and then directly used to produce the error message.
>
> With this patch the attempt to parse the driver type is skipped if not
> present to avoid changing the schema to forbid the empty driver element.
> ---
> src/conf/snapshot_conf.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index d8910c9..418987b 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -175,15 +175,17 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
> } else if (!def->format &&
> xmlStrEqual(cur->name, BAD_CAST "driver")) {
> char *driver = virXMLPropString(cur, "type");
> - def->format = virStorageFileFormatTypeFromString(driver);
> - if (def->format <= 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("unknown disk snapshot driver '%s'"),
> - driver);
> + if (driver) {
> + def->format = virStorageFileFormatTypeFromString(driver);
> + if (def->format <= 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unknown disk snapshot driver '%s'"),
> + driver);
> + VIR_FREE(driver);
> + goto cleanup;
> + }
> VIR_FREE(driver);
> - goto cleanup;
> }
> - VIR_FREE(driver);
> }
> }
So IIUC, this allows
<driver type="qemu"/>
<driver type="qemu" format="qcow2" />
but forbids
<driver format="qcow2" />
The domain XML parser, however, allows all 3 and 'type' will be auto
determined if omitted. IMHO it would be desirable to be consistent
between the parsers
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list