| Message ID | 1414589891-32736-3-git-send-email-den@openvz.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Oct 29, 2014 at 04:38:07PM +0300, Denis V. Lunev wrote: > Typically Parallels disk bundle consists of several images which are > glued by XML disk descriptor. Also XML hides inside several important > parameters which are not available in the image header. > > This patch allows to specify this XML file as a filename for an image > to open. It is allowed only to open Compressed images with the only > snapshot inside. No additional options are parsed at the moment. > > The code itself is dumb enough for a while. If XML file is specified, > the file is parsed and the image is reopened as bs->file to keep the > rest of the driver untouched. This would be changed later with more > features added. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > Acked-by: Roman Kagan <rkagan@parallels.com> > CC: Jeff Cody <jcody@redhat.com> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/parallels.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 226 insertions(+), 5 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index 4f9cd8d..201c0f1 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -27,6 +27,11 @@ > #include "block/block_int.h" > #include "qemu/module.h" > > +#if CONFIG_LIBXML2 > +#include <libxml/parser.h> > +#include <libxml/tree.h> > +#endif > + > /**************************************************************/ > > #define HEADER_MAGIC "WithoutFreeSpace" > @@ -34,6 +39,10 @@ > #define HEADER_VERSION 2 > #define HEADER_SIZE 64 > > +#define PARALLELS_XML 100 > +#define PARALLELS_IMAGE 101 > + > + > // always little-endian > struct parallels_header { > char magic[16]; // "WithoutFreeSpace" > @@ -59,6 +68,194 @@ typedef struct BDRVParallelsState { > unsigned int off_multiplier; > } BDRVParallelsState; > > +static int parallels_open_image(BlockDriverState *bs, Error **errp); You shouldn't need this forward declaration, if you put your new function parallels_open_xml() after parallels_open_image(). > + > +#if CONFIG_LIBXML2 > +static xmlNodePtr xml_find(xmlNode *node, const char *elem) > +{ > + xmlNode *child; > + > + for (child = node->xmlChildrenNode; child != NULL; child = child->next) { > + if (!xmlStrcmp(child->name, (const xmlChar *)elem) && > + child->type == XML_ELEMENT_NODE) { > + return child; > + } > + } > + return NULL; > +} > + > +static xmlNodePtr xml_seek(xmlNode *root, const char *elem) > +{ > + xmlNode *child = root; > + const char *path; > + char nodename[128]; > + int last = 0; > + > + path = elem; > + if (path[0] == '/') { > + path++; > + } > + if (path[0] == 0) { > + return NULL; > + } > + while (!last) { > + const char *p = strchr(path, '/'); > + int length; > + if (p == NULL) { > + length = strlen(path); > + last = 1; > + } else { > + length = p - path; > + } > + memcpy(nodename, path, length); > + nodename[length] = 0; It looks like "elem" is always controlled by us, and not passed by the user - will this always be the case? If not, this doesn't seem safe, with nodename being a local array of 128 bytes. How about using g_strdup() or g_strndup() here? > + child = xml_find(child, nodename); > + if (child == NULL) { > + return NULL; > + } > + path = ++p; > + } > + return child; > +} > + > +static const char *xml_get_text(xmlNode *node, const char *name) > +{ > + xmlNode *child; > + > + node = xml_seek(node, name); > + if (node == NULL) { > + return NULL; > + } > + > + for (child = node->xmlChildrenNode; child; child = child->next) { > + if (child->type == XML_TEXT_NODE) { > + return (const char *)child->content; > + } > + } > + return NULL; > +} > + > +static int parallels_open_xml(BlockDriverState *bs, int flags, Error **errp) > +{ > + int size, ret; > + xmlDoc *doc = NULL; > + xmlNode *root, *image; > + char *xml = NULL; > + const char *data; > + char image_path[PATH_MAX]; > + Error *local_err = NULL; > + > + ret = size = bdrv_getlength(bs->file); > + if (ret < 0) { > + goto fail; > + } > + /* XML file size should be resonable */ s/resonable/reasonable > + ret = -EFBIG; > + if (size > 65536) { > + goto fail; > + } > + > + xml = g_malloc(size + 1); > + > + ret = bdrv_pread(bs->file, 0, xml, size); > + if (ret != size) { > + goto fail; > + } > + xml[size] = 0; > + > + ret = -EINVAL; > + doc = xmlReadMemory(xml, size, NULL, NULL, > + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); > + if (doc == NULL) { > + goto fail; > + } > + root = xmlDocGetRootElement(doc); > + if (root == NULL) { > + goto fail; > + } > + image = xml_seek(root, "/StorageData/Storage/Image"); > + data = ""; /* make gcc happy */ What gcc warning are you trying to suppress here? > + for (size = 0; image != NULL; image = image->next) { > + if (image->type != XML_ELEMENT_NODE) { > + continue; > + } > + > + size++; > + data = xml_get_text(image, "Type"); > + if (data != NULL && strcmp(data, "Compressed")) { > + error_setg(errp, "Only compressed Parallels images are supported"); > + goto done; > + } > + > + data = xml_get_text(image, "File"); > + if (data == NULL) { > + goto fail; > + } > + } > + /* Images with more than 1 snapshots are not supported at the moment */ > + if (size != 1) { > + error_setg(errp, "Parallels images with snapshots are not supported"); > + goto done; > + } > + > + path_combine(image_path, sizeof(image_path), bs->file->filename, data); > + /* the caller (top layer bdrv_open) will close file for us if bs->file > + is changed. */ > + bs->file = NULL; > + > + ret = bdrv_open(&bs->file, image_path, NULL, NULL, flags | BDRV_O_PROTOCOL, > + NULL, &local_err); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not open '%s': %s", > + image_path, error_get_pretty(local_err)); > + error_free(local_err); error_get_pretty() is not NULL friendly - but I don't think it is an issue, because if (ret < 0) is true, then (local_err) should be set as well. Just an observation, I don't think you need to do anything here. > + } else { > + ret = parallels_open_image(bs, errp); > + } > + > +done: > + if (doc != NULL) { > + xmlFreeDoc(doc); > + } > + if (xml != NULL) { > + g_free(xml); > + } > + return ret; > + > +fail: > + error_setg(errp, "Failed to parse Parallels disk descriptor XML"); > + goto done; > +} > +#endif > + > +static int parallels_probe_xml(const uint8_t *buf, int buf_size) > +{ > + int score = 0; > +#if CONFIG_LIBXML2 > + xmlDoc *doc; > + xmlNode *root; > + > + doc = xmlReadMemory((const char *)buf, buf_size, NULL, NULL, > + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); > + if (doc == NULL) { > + return 0; /* This is not an XML, we don't care */ > + } > + > + root = xmlDocGetRootElement(doc); > + if (root == NULL) { > + goto done; > + } > + if (!xmlStrcmp(root->name, (const xmlChar *)"Parallels_disk_image")) { > + score = PARALLELS_XML; > + } > + > +done: > + xmlFreeDoc(doc); > +#endif > + return score; > +} > + > + > static int parallels_probe(const uint8_t *buf, int buf_size, const char *filename) > { > const struct parallels_header *ph = (const void *)buf; > @@ -69,13 +266,12 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam > if ((!memcmp(ph->magic, HEADER_MAGIC, 16) || > !memcmp(ph->magic, HEADER_MAGIC2, 16)) && > (le32_to_cpu(ph->version) == HEADER_VERSION)) > - return 100; > + return PARALLELS_IMAGE; > > - return 0; > + return parallels_probe_xml(buf, buf_size); > } > Hmm. So PARALLELS_XML is 100, and PARALLELS_IMAGE is 101 - and, this is used later in this patch, to differentiate between XML and non-XML images. This is somewhat abusing the .bdrv_probe(), I think - the intention of .bdrv_probe() is to return a confidence score between 0-100. I think you'd be better off just checking the magic in parallels_open() rather than overloading the .bdrv_probe() function. > -static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > - Error **errp) > +static int parallels_open_image(BlockDriverState *bs, Error **errp) > { > BDRVParallelsState *s = bs->opaque; > int i; > @@ -139,13 +335,38 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > return 0; > > fail_format: > - error_setg(errp, "Image not in Parallels format"); > + error_setg(errp, "Image is not in Parallels format"); > ret = -EINVAL; > fail: > g_free(s->catalog_bitmap); > return ret; > } > > +static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > + Error **errp) > +{ > + uint8_t buf[1024]; > + int score; > + > + score = bdrv_pread(bs->file, 0, buf, sizeof(buf)); > + if (score < 0) { > + return score; > + } > + > + score = parallels_probe(buf, (unsigned)score, NULL); > + if (score == PARALLELS_XML) { > +#if CONFIG_LIBXML2 > + return parallels_open_xml(bs, flags, errp); > +#endif > + } else if (score == PARALLELS_IMAGE) { > + return parallels_open_image(bs, errp); > + } > + > + error_setg(errp, "Image is not in Parallels format"); > + return -EINVAL; > +} > + > + > static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num) > { > BDRVParallelsState *s = bs->opaque; > -- > 1.9.1 >
On 05/11/14 05:12, Jeff Cody wrote: > On Wed, Oct 29, 2014 at 04:38:07PM +0300, Denis V. Lunev wrote: >> Typically Parallels disk bundle consists of several images which are >> glued by XML disk descriptor. Also XML hides inside several important >> parameters which are not available in the image header. >> >> This patch allows to specify this XML file as a filename for an image >> to open. It is allowed only to open Compressed images with the only >> snapshot inside. No additional options are parsed at the moment. >> >> The code itself is dumb enough for a while. If XML file is specified, >> the file is parsed and the image is reopened as bs->file to keep the >> rest of the driver untouched. This would be changed later with more >> features added. >> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> Acked-by: Roman Kagan <rkagan@parallels.com> >> CC: Jeff Cody <jcody@redhat.com> >> CC: Kevin Wolf <kwolf@redhat.com> >> CC: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> block/parallels.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 226 insertions(+), 5 deletions(-) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index 4f9cd8d..201c0f1 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -27,6 +27,11 @@ >> #include "block/block_int.h" >> #include "qemu/module.h" >> >> +#if CONFIG_LIBXML2 >> +#include <libxml/parser.h> >> +#include <libxml/tree.h> >> +#endif >> + >> /**************************************************************/ >> >> #define HEADER_MAGIC "WithoutFreeSpace" >> @@ -34,6 +39,10 @@ >> #define HEADER_VERSION 2 >> #define HEADER_SIZE 64 >> >> +#define PARALLELS_XML 100 >> +#define PARALLELS_IMAGE 101 >> + >> + >> // always little-endian >> struct parallels_header { >> char magic[16]; // "WithoutFreeSpace" >> @@ -59,6 +68,194 @@ typedef struct BDRVParallelsState { >> unsigned int off_multiplier; >> } BDRVParallelsState; >> >> +static int parallels_open_image(BlockDriverState *bs, Error **errp); > You shouldn't need this forward declaration, if you put your new > function parallels_open_xml() after parallels_open_image(). ok, fair enough. This is a rudiment from my previous internal version which has been implemented in the other way. >> + >> +#if CONFIG_LIBXML2 >> +static xmlNodePtr xml_find(xmlNode *node, const char *elem) >> +{ >> + xmlNode *child; >> + >> + for (child = node->xmlChildrenNode; child != NULL; child = child->next) { >> + if (!xmlStrcmp(child->name, (const xmlChar *)elem) && >> + child->type == XML_ELEMENT_NODE) { >> + return child; >> + } >> + } >> + return NULL; >> +} >> + >> +static xmlNodePtr xml_seek(xmlNode *root, const char *elem) >> +{ >> + xmlNode *child = root; >> + const char *path; >> + char nodename[128]; >> + int last = 0; >> + >> + path = elem; >> + if (path[0] == '/') { >> + path++; >> + } >> + if (path[0] == 0) { >> + return NULL; >> + } >> + while (!last) { >> + const char *p = strchr(path, '/'); >> + int length; >> + if (p == NULL) { >> + length = strlen(path); >> + last = 1; >> + } else { >> + length = p - path; >> + } >> + memcpy(nodename, path, length); >> + nodename[length] = 0; > It looks like "elem" is always controlled by us, and not passed by the > user - will this always be the case? > > If not, this doesn't seem safe, with nodename being a local array of > 128 bytes. How about using g_strdup() or g_strndup() here? yes, this constraint is used now and will be used in the future. XML structure is known and it is fixed. Here we just shortcut the navigation in the tree in a convenient way to pass entire path is one call. I think that we can switch to interface like static xmlNodePtr xml_seek(xmlNode *root, ...) and call it like xml_seek(root, "StorageData", "Storage", "Image", NULL); in this case we will be able to drop a lot of processing and parsing inside including strchr, memcpy, strlen etc >> + child = xml_find(child, nodename); >> + if (child == NULL) { >> + return NULL; >> + } >> + path = ++p; >> + } >> + return child; >> +} >> + >> +static const char *xml_get_text(xmlNode *node, const char *name) >> +{ >> + xmlNode *child; >> + >> + node = xml_seek(node, name); >> + if (node == NULL) { >> + return NULL; >> + } >> + >> + for (child = node->xmlChildrenNode; child; child = child->next) { >> + if (child->type == XML_TEXT_NODE) { >> + return (const char *)child->content; >> + } >> + } >> + return NULL; >> +} >> + >> +static int parallels_open_xml(BlockDriverState *bs, int flags, Error **errp) >> +{ >> + int size, ret; >> + xmlDoc *doc = NULL; >> + xmlNode *root, *image; >> + char *xml = NULL; >> + const char *data; >> + char image_path[PATH_MAX]; >> + Error *local_err = NULL; >> + >> + ret = size = bdrv_getlength(bs->file); >> + if (ret < 0) { >> + goto fail; >> + } >> + /* XML file size should be resonable */ > s/resonable/reasonable ok >> + ret = -EFBIG; >> + if (size > 65536) { >> + goto fail; >> + } >> + >> + xml = g_malloc(size + 1); >> + >> + ret = bdrv_pread(bs->file, 0, xml, size); >> + if (ret != size) { >> + goto fail; >> + } >> + xml[size] = 0; >> + >> + ret = -EINVAL; >> + doc = xmlReadMemory(xml, size, NULL, NULL, >> + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); >> + if (doc == NULL) { >> + goto fail; >> + } >> + root = xmlDocGetRootElement(doc); >> + if (root == NULL) { >> + goto fail; >> + } >> + image = xml_seek(root, "/StorageData/Storage/Image"); >> + data = ""; /* make gcc happy */ > What gcc warning are you trying to suppress here? use of uninitialized variable in the line below. path_combine(image_path, sizeof(image_path), bs->file->filename, data); which can not happen due to check size == 1 a line above. > >> + for (size = 0; image != NULL; image = image->next) { >> + if (image->type != XML_ELEMENT_NODE) { >> + continue; >> + } >> + >> + size++; >> + data = xml_get_text(image, "Type"); >> + if (data != NULL && strcmp(data, "Compressed")) { >> + error_setg(errp, "Only compressed Parallels images are supported"); >> + goto done; >> + } >> + >> + data = xml_get_text(image, "File"); >> + if (data == NULL) { >> + goto fail; >> + } >> + } >> + /* Images with more than 1 snapshots are not supported at the moment */ >> + if (size != 1) { >> + error_setg(errp, "Parallels images with snapshots are not supported"); >> + goto done; >> + } >> + >> + path_combine(image_path, sizeof(image_path), bs->file->filename, data); >> + /* the caller (top layer bdrv_open) will close file for us if bs->file >> + is changed. */ >> + bs->file = NULL; >> + >> + ret = bdrv_open(&bs->file, image_path, NULL, NULL, flags | BDRV_O_PROTOCOL, >> + NULL, &local_err); >> + if (ret < 0) { >> + error_setg_errno(errp, -ret, "Could not open '%s': %s", >> + image_path, error_get_pretty(local_err)); >> + error_free(local_err); > error_get_pretty() is not NULL friendly - but I don't think it is an > issue, because if (ret < 0) is true, then (local_err) should be set as > well. Just an observation, I don't think you need to do anything here. thank you for pointing this out >> + } else { >> + ret = parallels_open_image(bs, errp); >> + } >> + >> +done: >> + if (doc != NULL) { >> + xmlFreeDoc(doc); >> + } >> + if (xml != NULL) { >> + g_free(xml); >> + } >> + return ret; >> + >> +fail: >> + error_setg(errp, "Failed to parse Parallels disk descriptor XML"); >> + goto done; >> +} >> +#endif >> + >> +static int parallels_probe_xml(const uint8_t *buf, int buf_size) >> +{ >> + int score = 0; >> +#if CONFIG_LIBXML2 >> + xmlDoc *doc; >> + xmlNode *root; >> + >> + doc = xmlReadMemory((const char *)buf, buf_size, NULL, NULL, >> + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); >> + if (doc == NULL) { >> + return 0; /* This is not an XML, we don't care */ >> + } >> + >> + root = xmlDocGetRootElement(doc); >> + if (root == NULL) { >> + goto done; >> + } >> + if (!xmlStrcmp(root->name, (const xmlChar *)"Parallels_disk_image")) { >> + score = PARALLELS_XML; >> + } >> + >> +done: >> + xmlFreeDoc(doc); >> +#endif >> + return score; >> +} >> + >> + >> static int parallels_probe(const uint8_t *buf, int buf_size, const char *filename) >> { >> const struct parallels_header *ph = (const void *)buf; >> @@ -69,13 +266,12 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam >> if ((!memcmp(ph->magic, HEADER_MAGIC, 16) || >> !memcmp(ph->magic, HEADER_MAGIC2, 16)) && >> (le32_to_cpu(ph->version) == HEADER_VERSION)) >> - return 100; >> + return PARALLELS_IMAGE; >> >> - return 0; >> + return parallels_probe_xml(buf, buf_size); >> } >> > Hmm. So PARALLELS_XML is 100, and PARALLELS_IMAGE is 101 - and, this > is used later in this patch, to differentiate between XML and non-XML > images. This is somewhat abusing the .bdrv_probe(), I think - the > intention of .bdrv_probe() is to return a confidence score between > 0-100. > > I think you'd be better off just checking the magic in > parallels_open() rather than overloading the .bdrv_probe() function. > you are right. I have missed this thing. Need some thinking on this, but, anyway, this should be fixed. >> -static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> - Error **errp) >> +static int parallels_open_image(BlockDriverState *bs, Error **errp) >> { >> BDRVParallelsState *s = bs->opaque; >> int i; >> @@ -139,13 +335,38 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> return 0; >> >> fail_format: >> - error_setg(errp, "Image not in Parallels format"); >> + error_setg(errp, "Image is not in Parallels format"); >> ret = -EINVAL; >> fail: >> g_free(s->catalog_bitmap); >> return ret; >> } >> >> +static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> + Error **errp) >> +{ >> + uint8_t buf[1024]; >> + int score; >> + >> + score = bdrv_pread(bs->file, 0, buf, sizeof(buf)); >> + if (score < 0) { >> + return score; >> + } >> + >> + score = parallels_probe(buf, (unsigned)score, NULL); >> + if (score == PARALLELS_XML) { >> +#if CONFIG_LIBXML2 >> + return parallels_open_xml(bs, flags, errp); >> +#endif >> + } else if (score == PARALLELS_IMAGE) { >> + return parallels_open_image(bs, errp); >> + } >> + >> + error_setg(errp, "Image is not in Parallels format"); >> + return -EINVAL; >> +} >> + >> + >> static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num) >> { >> BDRVParallelsState *s = bs->opaque; >> -- >> 1.9.1 >>
diff --git a/block/parallels.c b/block/parallels.c index 4f9cd8d..201c0f1 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -27,6 +27,11 @@ #include "block/block_int.h" #include "qemu/module.h" +#if CONFIG_LIBXML2 +#include <libxml/parser.h> +#include <libxml/tree.h> +#endif + /**************************************************************/ #define HEADER_MAGIC "WithoutFreeSpace" @@ -34,6 +39,10 @@ #define HEADER_VERSION 2 #define HEADER_SIZE 64 +#define PARALLELS_XML 100 +#define PARALLELS_IMAGE 101 + + // always little-endian struct parallels_header { char magic[16]; // "WithoutFreeSpace" @@ -59,6 +68,194 @@ typedef struct BDRVParallelsState { unsigned int off_multiplier; } BDRVParallelsState; +static int parallels_open_image(BlockDriverState *bs, Error **errp); + +#if CONFIG_LIBXML2 +static xmlNodePtr xml_find(xmlNode *node, const char *elem) +{ + xmlNode *child; + + for (child = node->xmlChildrenNode; child != NULL; child = child->next) { + if (!xmlStrcmp(child->name, (const xmlChar *)elem) && + child->type == XML_ELEMENT_NODE) { + return child; + } + } + return NULL; +} + +static xmlNodePtr xml_seek(xmlNode *root, const char *elem) +{ + xmlNode *child = root; + const char *path; + char nodename[128]; + int last = 0; + + path = elem; + if (path[0] == '/') { + path++; + } + if (path[0] == 0) { + return NULL; + } + while (!last) { + const char *p = strchr(path, '/'); + int length; + if (p == NULL) { + length = strlen(path); + last = 1; + } else { + length = p - path; + } + memcpy(nodename, path, length); + nodename[length] = 0; + child = xml_find(child, nodename); + if (child == NULL) { + return NULL; + } + path = ++p; + } + return child; +} + +static const char *xml_get_text(xmlNode *node, const char *name) +{ + xmlNode *child; + + node = xml_seek(node, name); + if (node == NULL) { + return NULL; + } + + for (child = node->xmlChildrenNode; child; child = child->next) { + if (child->type == XML_TEXT_NODE) { + return (const char *)child->content; + } + } + return NULL; +} + +static int parallels_open_xml(BlockDriverState *bs, int flags, Error **errp) +{ + int size, ret; + xmlDoc *doc = NULL; + xmlNode *root, *image; + char *xml = NULL; + const char *data; + char image_path[PATH_MAX]; + Error *local_err = NULL; + + ret = size = bdrv_getlength(bs->file); + if (ret < 0) { + goto fail; + } + /* XML file size should be resonable */ + ret = -EFBIG; + if (size > 65536) { + goto fail; + } + + xml = g_malloc(size + 1); + + ret = bdrv_pread(bs->file, 0, xml, size); + if (ret != size) { + goto fail; + } + xml[size] = 0; + + ret = -EINVAL; + doc = xmlReadMemory(xml, size, NULL, NULL, + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); + if (doc == NULL) { + goto fail; + } + root = xmlDocGetRootElement(doc); + if (root == NULL) { + goto fail; + } + image = xml_seek(root, "/StorageData/Storage/Image"); + data = ""; /* make gcc happy */ + for (size = 0; image != NULL; image = image->next) { + if (image->type != XML_ELEMENT_NODE) { + continue; + } + + size++; + data = xml_get_text(image, "Type"); + if (data != NULL && strcmp(data, "Compressed")) { + error_setg(errp, "Only compressed Parallels images are supported"); + goto done; + } + + data = xml_get_text(image, "File"); + if (data == NULL) { + goto fail; + } + } + /* Images with more than 1 snapshots are not supported at the moment */ + if (size != 1) { + error_setg(errp, "Parallels images with snapshots are not supported"); + goto done; + } + + path_combine(image_path, sizeof(image_path), bs->file->filename, data); + /* the caller (top layer bdrv_open) will close file for us if bs->file + is changed. */ + bs->file = NULL; + + ret = bdrv_open(&bs->file, image_path, NULL, NULL, flags | BDRV_O_PROTOCOL, + NULL, &local_err); + if (ret < 0) { + error_setg_errno(errp, -ret, "Could not open '%s': %s", + image_path, error_get_pretty(local_err)); + error_free(local_err); + } else { + ret = parallels_open_image(bs, errp); + } + +done: + if (doc != NULL) { + xmlFreeDoc(doc); + } + if (xml != NULL) { + g_free(xml); + } + return ret; + +fail: + error_setg(errp, "Failed to parse Parallels disk descriptor XML"); + goto done; +} +#endif + +static int parallels_probe_xml(const uint8_t *buf, int buf_size) +{ + int score = 0; +#if CONFIG_LIBXML2 + xmlDoc *doc; + xmlNode *root; + + doc = xmlReadMemory((const char *)buf, buf_size, NULL, NULL, + XML_PARSE_NOERROR | XML_PARSE_NOWARNING); + if (doc == NULL) { + return 0; /* This is not an XML, we don't care */ + } + + root = xmlDocGetRootElement(doc); + if (root == NULL) { + goto done; + } + if (!xmlStrcmp(root->name, (const xmlChar *)"Parallels_disk_image")) { + score = PARALLELS_XML; + } + +done: + xmlFreeDoc(doc); +#endif + return score; +} + + static int parallels_probe(const uint8_t *buf, int buf_size, const char *filename) { const struct parallels_header *ph = (const void *)buf; @@ -69,13 +266,12 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam if ((!memcmp(ph->magic, HEADER_MAGIC, 16) || !memcmp(ph->magic, HEADER_MAGIC2, 16)) && (le32_to_cpu(ph->version) == HEADER_VERSION)) - return 100; + return PARALLELS_IMAGE; - return 0; + return parallels_probe_xml(buf, buf_size); } -static int parallels_open(BlockDriverState *bs, QDict *options, int flags, - Error **errp) +static int parallels_open_image(BlockDriverState *bs, Error **errp) { BDRVParallelsState *s = bs->opaque; int i; @@ -139,13 +335,38 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, return 0; fail_format: - error_setg(errp, "Image not in Parallels format"); + error_setg(errp, "Image is not in Parallels format"); ret = -EINVAL; fail: g_free(s->catalog_bitmap); return ret; } +static int parallels_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) +{ + uint8_t buf[1024]; + int score; + + score = bdrv_pread(bs->file, 0, buf, sizeof(buf)); + if (score < 0) { + return score; + } + + score = parallels_probe(buf, (unsigned)score, NULL); + if (score == PARALLELS_XML) { +#if CONFIG_LIBXML2 + return parallels_open_xml(bs, flags, errp); +#endif + } else if (score == PARALLELS_IMAGE) { + return parallels_open_image(bs, errp); + } + + error_setg(errp, "Image is not in Parallels format"); + return -EINVAL; +} + + static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num) { BDRVParallelsState *s = bs->opaque;