[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/16] block/parallels: support padded Parallels
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 05/16] block/parallels: support padded Parallels images |
Date: |
Mon, 15 Dec 2014 12:05:48 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 15.12.2014 um 09:27 hat Denis V. Lunev geschrieben:
> Unfortunately, old guest OSes do not align partitions to page size by
> default. This is true for Windows 2003 and Windows XP.
>
> For the time being Parallels was created an optimization for such OSes
> in its desktop product. Desktop users are not qualified enough to create
> properly aligned installations. Thus Parallels makes a blind guess
> on a customer behalf and creates so-called "padded" images if guest
> OS type is specified as WinXP, Win2k and Win2k3.
>
> "Padding" is a value which should be added to guest LBA to obtain
> sector number inside the image. This results in a shifted images.
> 0123 offset inside image (in 512 byte sectors)
> +-------
> +.012 guest data (512 byte sectors)
> +-------
> The information about this is available in DiskDescriptor.xml ONLY. There
> is no such data in the image header.
>
> There share of such images could be evaluated as 6-8% according to the
> statistics in my hands.
>
> This patch obtains proper value from XML and applies it on reading.
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> Acked-by: Roman Kagan <address@hidden>
> Reviewed-by: Jeff Cody <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> ---
> block/parallels.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index c22b91b..fedb009 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -63,6 +63,7 @@ typedef struct BDRVParallelsState {
> unsigned int tracks;
>
> unsigned int off_multiplier;
> + unsigned int padding;
> } BDRVParallelsState;
>
>
> @@ -235,6 +236,7 @@ static int parallels_open_xml(BlockDriverState *bs, int
> flags, Error **errp)
> const char *data;
> char image_path[PATH_MAX];
> Error *local_err = NULL;
> + BDRVParallelsState *s = bs->opaque;
>
> ret = size = bdrv_getlength(bs->file);
> if (ret < 0) {
> @@ -264,6 +266,19 @@ static int parallels_open_xml(BlockDriverState *bs, int
> flags, Error **errp)
> if (root == NULL) {
> goto fail;
> }
> +
> + data = xml_get_text(root, "Disk_Parameters", "Padding", NULL);
> + if (data != NULL) {
> + char *endptr;
> + unsigned long pad;
> +
> + pad = strtoul(data, &endptr, 0);
> + if ((endptr != NULL && *endptr != '\0') || pad > UINT_MAX) {
Can endptr even be NULL? Also, shouldn't you set errno = 0 before and
check it here?
> + goto fail;
> + }
> + s->padding = (uint32_t)pad;
s->padding is unsigned int, pad is unsigned long. Why the cast to
uint32_t here, which is different from both?
> + }
> +
> image = xml_seek(root, "StorageData", "Storage", "Image", NULL);
> data = ""; /* make gcc happy */
> for (size = 0; image != NULL; image = image->next) {
> @@ -365,6 +380,10 @@ static int64_t seek_to_sector(BlockDriverState *bs,
> int64_t sector_num)
> static int parallels_read(BlockDriverState *bs, int64_t sector_num,
> uint8_t *buf, int nb_sectors)
> {
> + BDRVParallelsState *s = bs->opaque;
> +
> + sector_num += s->padding;
No check needed here? bdrv_check_request() has checked sector_num and
nb_sectors against the image size, but now you can't rely on the result
any more.
> while (nb_sectors > 0) {
> int64_t position = seek_to_sector(bs, sector_num);
> if (position >= 0) {
Kevin
- [Qemu-devel] [PATCH v4 0/16] parallels format support improvements, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 07/16] parallels: change copyright information in the image header, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 04/16] iotests: simple parallels XML disk descriptor file test added, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 05/16] block/parallels: support padded Parallels images, Denis V. Lunev, 2014/12/15
- Re: [Qemu-devel] [PATCH 05/16] block/parallels: support padded Parallels images,
Kevin Wolf <=
- [Qemu-devel] [PATCH 06/16] iotests: padded parallels image test, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 03/16] iotests, parallels: quote TEST_IMG in 076 test to be path-safe, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 16/16] iotests: testcase parallels image with snapshots, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 02/16] block/parallels: allow to specify DiskDescriptor.xml instead of image file, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 15/16] block/parallels: support read-only parallels snapshots, Denis V. Lunev, 2014/12/15
- [Qemu-devel] [PATCH 13/16] block/parallels: read disk size from XML if DiskDescriptor.xml is passed, Denis V. Lunev, 2014/12/15