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

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.

commit eda7654e5e63aa4a64cdcc7f38d8e40c14197af1 Author: Cole Robinson <crobinso(a)redhat.com&gt; 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'"),