[libvirt] [PATCH 0/3] Fix issues with disk backend partition recognition
https://bugzilla.redhat.com/show_bug.cgi?id=1439132 As described in patches. John Ferlan (3): storage: Introduce virStorageBackendZeroDeviceHeader logical: Use virStorageBackendZeroDeviceHeader to clear header storage: Resolve issues with disk partition build/start checks src/storage/storage_backend_disk.c | 4 ++++ src/storage/storage_backend_logical.c | 44 +---------------------------------- src/storage/storage_util.c | 28 ++++++++++++++++++---- src/storage/storage_util.h | 4 ++++ 4 files changed, 33 insertions(+), 47 deletions(-) -- 2.9.3
Create a wrapper/helper that can be used to call the storage backend wipe helper - storageBackendVolWipeLocalFile for future use by logical and disk backends to clear out the partition table rather than having each open code the same algorithm. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 18 ++++++++++++++++++ src/storage/storage_util.h | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 8e25984..784df7a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -4087,3 +4087,21 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, return found; } + + +/* + * @path: Path to the device to initialize + * @size: Size to be cleared + * + * Clear out device header for the purpose of being able to write + * some sort of partition to the device. + * + * Returns 0 on success, -1 on failure with error message set + */ +int +virStorageBackendZeroDeviceHeader(const char *path, + unsigned long long size) +{ + return storageBackendVolWipeLocalFile(path, VIR_STORAGE_VOL_WIPE_ALG_ZERO, + size); +} diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index 602d3a0..756d140 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -167,4 +167,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, int virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, uint32_t scanhost); +int +virStorageBackendZeroDeviceHeader(const char *path, + unsigned long long size); + #endif /* __VIR_STORAGE_UTIL_H__ */ -- 2.9.3
On Fri, Apr 07, 2017 at 07:49:57 -0400, John Ferlan wrote:
Create a wrapper/helper that can be used to call the storage backend wipe helper - storageBackendVolWipeLocalFile for future use by logical and disk backends to clear out the partition table rather than having each open code the same algorithm.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 18 ++++++++++++++++++ src/storage/storage_util.h | 4 ++++ 2 files changed, 22 insertions(+)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 8e25984..784df7a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -4087,3 +4087,21 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool,
return found; } + + +/* + * @path: Path to the device to initialize + * @size: Size to be cleared + * + * Clear out device header for the purpose of being able to write + * some sort of partition to the device. + * + * Returns 0 on success, -1 on failure with error message set + */ +int +virStorageBackendZeroDeviceHeader(const char *path, + unsigned long long size) +{ + return storageBackendVolWipeLocalFile(path, VIR_STORAGE_VOL_WIPE_ALG_ZERO, + size);
Do you plan on adding stuff here? otherwise it's quite useless.
+} diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index 602d3a0..756d140 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -167,4 +167,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, int virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, uint32_t scanhost);
+int +virStorageBackendZeroDeviceHeader(const char *path, + unsigned long long size); + #endif /* __VIR_STORAGE_UTIL_H__ */ -- 2.9.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/07/2017 07:54 AM, Peter Krempa wrote:
On Fri, Apr 07, 2017 at 07:49:57 -0400, John Ferlan wrote:
Create a wrapper/helper that can be used to call the storage backend wipe helper - storageBackendVolWipeLocalFile for future use by logical and disk backends to clear out the partition table rather than having each open code the same algorithm.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 18 ++++++++++++++++++ src/storage/storage_util.h | 4 ++++ 2 files changed, 22 insertions(+)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 8e25984..784df7a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -4087,3 +4087,21 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool,
return found; } + + +/* + * @path: Path to the device to initialize + * @size: Size to be cleared + * + * Clear out device header for the purpose of being able to write + * some sort of partition to the device. + * + * Returns 0 on success, -1 on failure with error message set + */ +int +virStorageBackendZeroDeviceHeader(const char *path, + unsigned long long size) +{ + return storageBackendVolWipeLocalFile(path, VIR_STORAGE_VOL_WIPE_ALG_ZERO, + size);
Do you plan on adding stuff here? otherwise it's quite useless.
You mean like merging patch 1 and 2? I started that way, but waffled back and forth finally deciding upon the one thing at a time approach. John
+} diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index 602d3a0..756d140 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -167,4 +167,8 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, int virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, uint32_t scanhost);
+int +virStorageBackendZeroDeviceHeader(const char *path, + unsigned long long size); + #endif /* __VIR_STORAGE_UTIL_H__ */ -- 2.9.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Apr 07, 2017 at 07:58:11 -0400, John Ferlan wrote:
On 04/07/2017 07:54 AM, Peter Krempa wrote:
On Fri, Apr 07, 2017 at 07:49:57 -0400, John Ferlan wrote:
Create a wrapper/helper that can be used to call the storage backend wipe helper - storageBackendVolWipeLocalFile for future use by logical and disk backends to clear out the partition table rather than having each open code the same algorithm.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 18 ++++++++++++++++++ src/storage/storage_util.h | 4 ++++ 2 files changed, 22 insertions(+)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 8e25984..784df7a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -4087,3 +4087,21 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool,
return found; } + + +/* + * @path: Path to the device to initialize + * @size: Size to be cleared + * + * Clear out device header for the purpose of being able to write + * some sort of partition to the device. + * + * Returns 0 on success, -1 on failure with error message set + */ +int +virStorageBackendZeroDeviceHeader(const char *path, + unsigned long long size) +{ + return storageBackendVolWipeLocalFile(path, VIR_STORAGE_VOL_WIPE_ALG_ZERO, + size);
Do you plan on adding stuff here? otherwise it's quite useless.
You mean like merging patch 1 and 2? I started that way, but waffled back and forth finally deciding upon the one thing at a time approach.
No. I meant using storageBackendVolWipeLocalFile directly. In fact this is kind of useful since storageBackendVolWipeLocalFile is not exported, so this patch hacks-around that fact, by exporting this new helper. That makes kind of sense. ACK
Rather than open code it, use the new function which uses the wipe algorithm Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_logical.c | 44 +---------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index d87aaf0..3aaad04 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -92,9 +92,6 @@ virStorageBackendLogicalRemoveDevice(const char *path) static int virStorageBackendLogicalInitializeDevice(const char *path) { - int fd = -1; - char zeros[4 * PV_BLANK_SECTOR_SIZE] = {0}; - off_t size; int ret = -1; virCommandPtr pvcmd = NULL; @@ -103,46 +100,8 @@ virStorageBackendLogicalInitializeDevice(const char *path) * a whole disk as a PV. So we just blank them out regardless * rather than trying to figure out if we're a disk or partition */ - if ((fd = open(path, O_WRONLY)) < 0) { - virReportSystemError(errno, _("cannot open device '%s'"), path); + if (virStorageBackendZeroDeviceHeader(path, 4 * PV_BLANK_SECTOR_SIZE) < 0) return -1; - } - - if ((size = lseek(fd, 0, SEEK_END)) == (off_t)-1) { - virReportSystemError(errno, - _("failed to seek to end of %s"), path); - goto cleanup; - } - - if (size < 4 * PV_BLANK_SECTOR_SIZE) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("cannot initialize '%s' detected size='%zd' less " - "than minimum required='%d"), - path, (ssize_t) size, 4 * PV_BLANK_SECTOR_SIZE); - goto cleanup; - } - if ((size = lseek(fd, 0, SEEK_SET)) == (off_t)-1) { - virReportSystemError(errno, - _("failed to seek to start of %s"), path); - goto cleanup; - } - - if (safewrite(fd, zeros, sizeof(zeros)) < 0) { - virReportSystemError(errno, _("cannot clear device header of '%s'"), - path); - goto cleanup; - } - - if (fsync(fd) < 0) { - virReportSystemError(errno, _("cannot flush header of device'%s'"), - path); - goto cleanup; - } - - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("cannot close device '%s'"), path); - goto cleanup; - } /* * Initialize the physical volume because vgcreate is not @@ -155,7 +114,6 @@ virStorageBackendLogicalInitializeDevice(const char *path) ret = 0; cleanup: - VIR_FORCE_CLOSE(fd); virCommandFree(pvcmd); return ret; -- 2.9.3
On Fri, Apr 07, 2017 at 07:49:58 -0400, John Ferlan wrote:
Rather than open code it, use the new function which uses the wipe algorithm
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_logical.c | 44 +---------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-)
ACK
https://bugzilla.redhat.com/show_bug.cgi?id=1439132 Fix some issues discovered when building between multiple different supported disk partition types. 1. The "dvh" and "pc98" were specifically checked during the blkid partition processing in commit id 'a48c674fb', but commit id 'a4cb4a74f' really messed things up by missing an else condition causing PARTEDFindLabel to always return DIFFERENT. 2. Add the "bsd" to the list of format types to not checked during blkid processing even though it supposedly knows the format - for some (now unknown) reason it's returning partiion not found. So let's just let PARTED handle that too. 3. During 'matrix' testing of all possible combinations I found that if device is formated with "gpt" first, then an attempt is made to format using "mac", a startup will fail. By adding a clearing of the first 2048 bytes of the device (similar to the logical pool code), the issue is resolved. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_disk.c | 4 ++++ src/storage/storage_util.c | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 39371f2..ce40ddd 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -496,6 +496,10 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, } if (ok_to_mklabel) { + if (virStorageBackendZeroDeviceHeader(pool->def->source.devices[0].path, + 4 * SECTOR_SIZE) < 0) + goto error; + /* eg parted /dev/sda mklabel --script msdos */ if (format == VIR_STORAGE_POOL_DISK_UNKNOWN) format = pool->def->source.format = VIR_STORAGE_POOL_DISK_DOS; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 784df7a..812429e 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3034,10 +3034,12 @@ virStorageBackendBLKIDFindPart(blkid_probe probe, /* A blkid_known_pttype on "dvh" and "pc98" returns a failure; * however, the blkid_do_probe for "dvh" returns "sgi" and - * for "pc98" it returns "dos". So since those will cause problems + * for "pc98" it returns "dos". Although "bsd" is recognized, + * it seems that the parted created partition table is not being + * properly recogized. Since each of these will cause problems * with startup comparison, let's just treat them as UNKNOWN causing * the caller to fallback to using PARTED */ - if (STREQ(format, "dvh") || STREQ(format, "pc98")) + if (STREQ(format, "dvh") || STREQ(format, "pc98") || STREQ(format, "bsd")) return VIR_STORAGE_BLKID_PROBE_UNKNOWN; /* Make sure we're doing a partitions probe from the start */ @@ -3245,8 +3247,8 @@ virStorageBackendPARTEDFindLabel(const char *device, /* Does the on disk match what the pool desired? */ if (STREQ(start, format)) ret = VIR_STORAGE_PARTED_MATCH; - - ret = VIR_STORAGE_PARTED_DIFFERENT; + else + ret = VIR_STORAGE_PARTED_DIFFERENT; cleanup: virCommandFree(cmd); -- 2.9.3
On Fri, Apr 07, 2017 at 07:49:59 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
Fix some issues discovered when building between multiple different supported disk partition types.
1. The "dvh" and "pc98" were specifically checked during the blkid partition processing in commit id 'a48c674fb', but commit id 'a4cb4a74f' really messed things up by missing an else condition causing PARTEDFindLabel to always return DIFFERENT.
2. Add the "bsd" to the list of format types to not checked during blkid processing even though it supposedly knows the format - for some (now unknown) reason it's returning partiion not found. So let's just let PARTED handle that too.
3. During 'matrix' testing of all possible combinations I found that if device is formated with "gpt" first, then an attempt is made to format using "mac", a startup will fail. By adding a clearing of the first 2048 bytes of the device (similar to the logical pool code), the issue is resolved.
Looks like 3 separate bugs to me. Why are all the fixes in a single commit?
On 04/07/2017 08:08 AM, Peter Krempa wrote:
On Fri, Apr 07, 2017 at 07:49:59 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
Fix some issues discovered when building between multiple different supported disk partition types.
1. The "dvh" and "pc98" were specifically checked during the blkid partition processing in commit id 'a48c674fb', but commit id 'a4cb4a74f' really messed things up by missing an else condition causing PARTEDFindLabel to always return DIFFERENT.
2. Add the "bsd" to the list of format types to not checked during blkid processing even though it supposedly knows the format - for some (now unknown) reason it's returning partiion not found. So let's just let PARTED handle that too.
3. During 'matrix' testing of all possible combinations I found that if device is formated with "gpt" first, then an attempt is made to format using "mac", a startup will fail. By adding a clearing of the first 2048 bytes of the device (similar to the logical pool code), the issue is resolved.
Looks like 3 separate bugs to me. Why are all the fixes in a single commit?
There's just one bz - I can separate each if desired. John
https://bugzilla.redhat.com/show_bug.cgi?id=1439132 During 'matrix' testing of all possible combinations I found that if device is formated with "gpt" first, then an attempt is made to format using "mac", a startup will fail. By adding a clearing of the first 2048 bytes of the device (similar to the logical pool code), the issue is resolved. Signed-off-by: John Ferlan <jferlan@redhat.com> --- Difference to v1 - just split it out and sent separately with it's own commit message. src/storage/storage_backend_disk.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 39371f2..ce40ddd 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -496,6 +496,10 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, } if (ok_to_mklabel) { + if (virStorageBackendZeroDeviceHeader(pool->def->source.devices[0].path, + 4 * SECTOR_SIZE) < 0) + goto error; + /* eg parted /dev/sda mklabel --script msdos */ if (format == VIR_STORAGE_POOL_DISK_UNKNOWN) format = pool->def->source.format = VIR_STORAGE_POOL_DISK_DOS; -- 2.9.3
On Fri, Apr 07, 2017 at 09:52:09 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
During 'matrix' testing of all possible combinations I found that if device is formated with "gpt" first, then an attempt is made to format using "mac", a startup will fail. By adding a clearing of the first 2048 bytes of the device (similar to the logical pool code), the issue is resolved.
I'm not a fan of this and neither thing this is robust enough: The problem here is that apparently the "mac" table fits into the first block on the disk. Since the GPT disklabel is stored at LBA address 1 it is not overwritten at all. Thus it's apparent that the detection tool then prefers GPT over a older disklabel. The GPT disklabel has also a secondary copy at the last LBA of the disk. As for robustness: SECTOR_SIZE is defined as 512 in our code, thus this code nukes 2k of data. For devices with 4k sectors which are already available this won't help at all, since LBA 0 will still contain the protective MBR, and the 3,5k left of LBA 0 will be unused. GPT disklabel still will start at LBA 1. Since I'm not a fan of similar code when nuking physical volumes I'd rather not see this stuff to spread.
On Fri, Apr 07, 2017 at 04:27:40PM +0200, Peter Krempa wrote:
On Fri, Apr 07, 2017 at 09:52:09 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
During 'matrix' testing of all possible combinations I found that if device is formated with "gpt" first, then an attempt is made to format using "mac", a startup will fail. By adding a clearing of the first 2048 bytes of the device (similar to the logical pool code), the issue is resolved.
I'm not a fan of this and neither thing this is robust enough:
The problem here is that apparently the "mac" table fits into the first block on the disk. Since the GPT disklabel is stored at LBA address 1 it is not overwritten at all. Thus it's apparent that the detection tool then prefers GPT over a older disklabel.
The GPT disklabel has also a secondary copy at the last LBA of the disk.
As for robustness: SECTOR_SIZE is defined as 512 in our code, thus this code nukes 2k of data. For devices with 4k sectors which are already available this won't help at all, since LBA 0 will still contain the protective MBR, and the 3,5k left of LBA 0 will be unused. GPT disklabel still will start at LBA 1.
Personally I'd forget about sectors and just nuke 1 MB of data at start and end of the device. The time for 2k vs 1 MB is negligible. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On 04/07/2017 10:31 AM, Daniel P. Berrange wrote:
On Fri, Apr 07, 2017 at 04:27:40PM +0200, Peter Krempa wrote:
On Fri, Apr 07, 2017 at 09:52:09 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
During 'matrix' testing of all possible combinations I found that if device is formated with "gpt" first, then an attempt is made to format using "mac", a startup will fail. By adding a clearing of the first 2048 bytes of the device (similar to the logical pool code), the issue is resolved.
I'm not a fan of this and neither thing this is robust enough:
The problem here is that apparently the "mac" table fits into the first block on the disk. Since the GPT disklabel is stored at LBA address 1 it is not overwritten at all. Thus it's apparent that the detection tool then prefers GPT over a older disklabel.
The GPT disklabel has also a secondary copy at the last LBA of the disk.
As for robustness: SECTOR_SIZE is defined as 512 in our code, thus this code nukes 2k of data. For devices with 4k sectors which are already available this won't help at all, since LBA 0 will still contain the protective MBR, and the 3,5k left of LBA 0 will be unused. GPT disklabel still will start at LBA 1.
Personally I'd forget about sectors and just nuke 1 MB of data at start and end of the device. The time for 2k vs 1 MB is negligible.
OK - so I've pushed patches 4 & 5 and will rework the first 3 ... series coming soon the a mailing list near you. John
https://bugzilla.redhat.com/show_bug.cgi?id=1439132 Fix some issues discovered when building between multiple different supported disk partition types. Commit id 'a48c674fb' added a check for format types "dvh" and "pc98" to use the parted print processing instead of using blkid processing in order to validate the label on the disk was what is expected for disk pool startup. However, commit id 'a4cb4a74f' really messed things up by missing an else condition causing PARTEDFindLabel to always return DIFFERENT. Signed-off-by: John Ferlan <jferlan@redhat.com> --- Difference to v1: Pulled out specific patch and sent separately src/storage/storage_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 784df7a..4ced81e 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3245,8 +3245,8 @@ virStorageBackendPARTEDFindLabel(const char *device, /* Does the on disk match what the pool desired? */ if (STREQ(start, format)) ret = VIR_STORAGE_PARTED_MATCH; - - ret = VIR_STORAGE_PARTED_DIFFERENT; + else + ret = VIR_STORAGE_PARTED_DIFFERENT; cleanup: virCommandFree(cmd); -- 2.9.3
On Fri, Apr 07, 2017 at 09:52:59 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
Fix some issues discovered when building between multiple different supported disk partition types.
Looks like a single issue.
Commit id 'a48c674fb' added a check for format types "dvh" and "pc98" to use the parted print processing instead of using blkid processing in order to validate the label on the disk was what is expected for disk pool startup. However, commit id 'a4cb4a74f' really messed things up by missing an else condition causing PARTEDFindLabel to always return DIFFERENT.
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
Difference to v1: Pulled out specific patch and sent separately
src/storage/storage_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 784df7a..4ced81e 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3245,8 +3245,8 @@ virStorageBackendPARTEDFindLabel(const char *device, /* Does the on disk match what the pool desired? */ if (STREQ(start, format)) ret = VIR_STORAGE_PARTED_MATCH; - - ret = VIR_STORAGE_PARTED_DIFFERENT; + else + ret = VIR_STORAGE_PARTED_DIFFERENT;
cleanup: virCommandFree(cmd);
ACK
https://bugzilla.redhat.com/show_bug.cgi?id=1439132 Add "bsd" to the list of format types to not checked during blkid processing even though it supposedly knows the format - for some (now unknown) reason it's returning partiion not found. So let's just let PARTED handle "bsd" too. Signed-off-by: John Ferlan <jferlan@redhat.com> --- Difference to v1: Just pull out and send separately. src/storage/storage_util.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 4ced81e..812429e 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3034,10 +3034,12 @@ virStorageBackendBLKIDFindPart(blkid_probe probe, /* A blkid_known_pttype on "dvh" and "pc98" returns a failure; * however, the blkid_do_probe for "dvh" returns "sgi" and - * for "pc98" it returns "dos". So since those will cause problems + * for "pc98" it returns "dos". Although "bsd" is recognized, + * it seems that the parted created partition table is not being + * properly recogized. Since each of these will cause problems * with startup comparison, let's just treat them as UNKNOWN causing * the caller to fallback to using PARTED */ - if (STREQ(format, "dvh") || STREQ(format, "pc98")) + if (STREQ(format, "dvh") || STREQ(format, "pc98") || STREQ(format, "bsd")) return VIR_STORAGE_BLKID_PROBE_UNKNOWN; /* Make sure we're doing a partitions probe from the start */ -- 2.9.3
On Fri, Apr 07, 2017 at 09:53:40 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439132
Add "bsd" to the list of format types to not checked during blkid processing even though it supposedly knows the format - for some (now unknown) reason it's returning partiion not found. So let's
s/partiion/partition/ Also it's not really a partition, it's a disklabel or partition table.
just let PARTED handle "bsd" too.
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
Difference to v1: Just pull out and send separately.
src/storage/storage_util.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 4ced81e..812429e 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3034,10 +3034,12 @@ virStorageBackendBLKIDFindPart(blkid_probe probe,
/* A blkid_known_pttype on "dvh" and "pc98" returns a failure; * however, the blkid_do_probe for "dvh" returns "sgi" and - * for "pc98" it returns "dos". So since those will cause problems + * for "pc98" it returns "dos". Although "bsd" is recognized, + * it seems that the parted created partition table is not being + * properly recogized. Since each of these will cause problems * with startup comparison, let's just treat them as UNKNOWN causing * the caller to fallback to using PARTED */ - if (STREQ(format, "dvh") || STREQ(format, "pc98")) + if (STREQ(format, "dvh") || STREQ(format, "pc98") || STREQ(format, "bsd")) return VIR_STORAGE_BLKID_PROBE_UNKNOWN;
/* Make sure we're doing a partitions probe from the start */
ACK
participants (3)
-
Daniel P. Berrange -
John Ferlan -
Peter Krempa