[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Don't squash file format on volume refresh



On Thu, Apr 02, 2009 at 02:01:02PM -0400, Cole Robinson wrote:
> A while back, code was added to
> storage_backend.c:virStorageBackendUpdateVolTargetInfoFD to try to
> determine the volume format for iscsi volumes. Unfortunately this
> function is also called for refreshing file volumes, which has the
> effect of setting the format as 'raw' whenever a volume is refreshed
> (ex. after calling virsh vol-info).
> 
> The attached patch moves the offending code into a wrapper function in
> the scsi driver. I haven't played with the scsi support yet so this is
> untested though it's largely code movement.

Ahhh, this is also what breaks the storage_backend_disk driver
with it blowing away the partition types for each partition.

ACK


> commit eda7654e5e63aa4a64cdcc7f38d8e40c14197af1
> Author: Cole Robinson <crobinso redhat com>
> Date:   Thu Apr 2 13:37:40 2009 -0400
> 
>     Don't lose file format info on volume refresh.
> 
> diff --git a/src/storage_backend.c b/src/storage_backend.c
> index d9ffaee..b154140 100644
> --- a/src/storage_backend.c
> +++ b/src/storage_backend.c
> @@ -156,37 +156,6 @@ virStorageBackendUpdateVolInfo(virConnectPtr conn,
>      return 0;
>  }
>  
> -struct diskType {
> -    int part_table_type;
> -    unsigned short offset;
> -    unsigned short length;
> -    unsigned long long magic;
> -};
> -
> -static struct diskType const disk_types[] = {
> -    { VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CULL },
> -    { VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645ULL },
> -    { VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BULL },
> -    { VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245ULL },
> -    { VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557ULL },
> -    { VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAULL },
> -    /*
> -     * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so
> -     * we can't use that.  At the moment I'm relying on the "dummy" IPL
> -     * bootloader data that comes from parted.  Luckily, the chances of running
> -     * into a pc98 machine running libvirt are approximately nil.
> -     */
> -    /*{ 0x1fe, 2, 0xAA55UL },*/
> -    { VIR_STORAGE_POOL_DISK_PC98, 0x0,   8, 0x314C5049000000CBULL },
> -    /*
> -     * NOTE: the order is important here; some other disk types (like GPT and
> -     * and PC98) also have 0x55AA at this offset.  For that reason, the DOS
> -     * one must be the last one.
> -     */
> -    { VIR_STORAGE_POOL_DISK_DOS,  0x1fe, 2, 0xAA55ULL },
> -    { -1,                         0x0,   0, 0x0ULL },
> -};
> -
>  int
>  virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn,
>                                         virStorageVolTargetPtr target,
> @@ -244,41 +213,6 @@ virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn,
>          }
>      }
>  
> -    if (S_ISBLK(sb.st_mode)) {
> -        off_t start;
> -        int i;
> -        unsigned char buffer[1024];
> -        ssize_t bytes;
> -
> -        /* make sure to set the target format "unknown" to begin with */
> -        target->format = VIR_STORAGE_POOL_DISK_UNKNOWN;
> -
> -        start = lseek(fd, 0, SEEK_SET);
> -        if (start < 0) {
> -            virReportSystemError(conn, errno,
> -                                 _("cannot seek to beginning of file '%s'"),
> -                                 target->path);
> -            return -1;
> -        }
> -        bytes = saferead(fd, buffer, sizeof(buffer));
> -        if (bytes < 0) {
> -            virReportSystemError(conn, errno,
> -                                 _("cannot read beginning of file '%s'"),
> -                                 target->path);
> -            return -1;
> -        }
> -
> -        for (i = 0; disk_types[i].part_table_type != -1; i++) {
> -            if (disk_types[i].offset + disk_types[i].length > bytes)
> -                continue;
> -            if (memcmp(buffer+disk_types[i].offset, &disk_types[i].magic,
> -                disk_types[i].length) == 0) {
> -                target->format = disk_types[i].part_table_type;
> -                break;
> -            }
> -        }
> -    }
> -
>      target->perms.mode = sb.st_mode & S_IRWXUGO;
>      target->perms.uid = sb.st_uid;
>      target->perms.gid = sb.st_gid;
> diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c
> index a962d1c..b3e6180 100644
> --- a/src/storage_backend_scsi.c
> +++ b/src/storage_backend_scsi.c
> @@ -100,6 +100,92 @@ out:
>      return retval;
>  }
>  
> +struct diskType {
> +    int part_table_type;
> +    unsigned short offset;
> +    unsigned short length;
> +    unsigned long long magic;
> +};
> +
> +static struct diskType const disk_types[] = {
> +    { VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CULL },
> +    { VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645ULL },
> +    { VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BULL },
> +    { VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245ULL },
> +    { VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557ULL },
> +    { VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAULL },
> +    /*
> +     * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so
> +     * we can't use that.  At the moment I'm relying on the "dummy" IPL
> +     * bootloader data that comes from parted.  Luckily, the chances of running
> +     * into a pc98 machine running libvirt are approximately nil.
> +     */
> +    /*{ 0x1fe, 2, 0xAA55UL },*/
> +    { VIR_STORAGE_POOL_DISK_PC98, 0x0,   8, 0x314C5049000000CBULL },
> +    /*
> +     * NOTE: the order is important here; some other disk types (like GPT and
> +     * and PC98) also have 0x55AA at this offset.  For that reason, the DOS
> +     * one must be the last one.
> +     */
> +    { VIR_STORAGE_POOL_DISK_DOS,  0x1fe, 2, 0xAA55ULL },
> +    { -1,                         0x0,   0, 0x0ULL },
> +};
> +
> +static int
> +virStorageBackendSCSIUpdateVolTargetInfo(virConnectPtr conn,
> +                                         virStorageVolTargetPtr target,
> +                                         unsigned long long *allocation,
> +                                         unsigned long long *capacity)
> +{
> +    int fd, i;
> +    off_t start;
> +    unsigned char buffer[1024];
> +    ssize_t bytes;
> +
> +    if ((fd = open(target->path, O_RDONLY)) < 0) {
> +        virReportSystemError(conn, errno,
> +                             _("cannot open volume '%s'"),
> +                             target->path);
> +        return -1;
> +    }
> +
> +    if (virStorageBackendUpdateVolTargetInfoFD(conn,
> +                                               target,
> +                                               fd,
> +                                               allocation,
> +                                               capacity) < 0)
> +        return -1;
> +
> +    /* make sure to set the target format "unknown" to begin with */
> +    target->format = VIR_STORAGE_POOL_DISK_UNKNOWN;
> +
> +    start = lseek(fd, 0, SEEK_SET);
> +    if (start < 0) {
> +        virReportSystemError(conn, errno,
> +                             _("cannot seek to beginning of file '%s'"),
> +                             target->path);
> +        return -1;
> +    }
> +    bytes = saferead(fd, buffer, sizeof(buffer));
> +    if (bytes < 0) {
> +        virReportSystemError(conn, errno,
> +                             _("cannot read beginning of file '%s'"),
> +                             target->path);
> +        return -1;
> +    }
> +
> +    for (i = 0; disk_types[i].part_table_type != -1; i++) {
> +        if (disk_types[i].offset + disk_types[i].length > bytes)
> +            continue;
> +        if (memcmp(buffer+disk_types[i].offset, &disk_types[i].magic,
> +            disk_types[i].length) == 0) {
> +            target->format = disk_types[i].part_table_type;
> +            break;
> +        }
> +    }
> +
> +    return 0;
> +}
>  
>  static int
>  virStorageBackendSCSINewLun(virConnectPtr conn,
> @@ -160,10 +246,10 @@ virStorageBackendSCSINewLun(virConnectPtr conn,
>          goto free_vol;
>      }
>  
> -    if (virStorageBackendUpdateVolTargetInfo(conn,
> -                                             &vol->target,
> -                                             &vol->allocation,
> -                                             &vol->capacity) < 0) {
> +    if (virStorageBackendSCSIUpdateVolTargetInfo(conn,
> +                                                 &vol->target,
> +                                                 &vol->allocation,
> +                                                 &vol->capacity) < 0) {
>  
>          virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
>                                _("Failed to update volume for '%s'"),


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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]