Re: [libvirt] [PATCH 03/11] storage: Add partition type checks for BLKID probing

On 12/15/2016 10:42 PM, John Ferlan wrote: > A device may be formatted using some sort of disk partition format type. > We can check that using the blkid_ API's as well - so alter the logic to > allow checking the device for both a filesystem and a disk partition. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/storage/storage_backend.c | 177 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 138 insertions(+), 39 deletions(-) > > diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c > index 108f2b9..065f2ef 100644 > --- a/src/storage/storage_backend.c > +++ b/src/storage/storage_backend.c > @@ -2639,37 +2639,117 @@ virStorageBackendFindGlusterPoolSources(const char *host ATTRIBUTE_UNUSED, > > > #if WITH_BLKID > + > +typedef enum { > + VIR_STORAGE_BLKID_PROBE_ERROR = -1, > + VIR_STORAGE_BLKID_PROBE_UNDEFINED, /* Nothing found */ > + VIR_STORAGE_BLKID_PROBE_UNKNOWN, /* Don't know libvirt fs/part type */ > + VIR_STORAGE_BLKID_PROBE_MATCH, /* Matches the on disk format */ > + VIR_STORAGE_BLKID_PROBE_DIFFERENT, /* Format doesn't match on disk format */ > +} virStorageBackendBLKIDProbeResult; > + > +/* > + * Utility function to probe for a file system on the device using the > + * blkid "superblock" (e.g. default) APIs. > + * > + * NB: In general this helper will handle the virStoragePoolFormatFileSystem > + * format types; however, if called from the Disk path, the initial fstype > + * check will fail forcing the usage of the ProbePart helper. > + * > + * Returns virStorageBackendBLKIDProbeResult enum > + */ > +static virStorageBackendBLKIDProbeResult > +virStorageBackendBLKIDProbeFS(blkid_probe probe, > + const char *device, > + const char *format) > +{ > + const char *fstype = NULL; > + > + /* Make sure we're doing a superblock probe from the start */ > + blkid_probe_enable_superblocks(probe, true); > + blkid_probe_reset_superblocks_filter(probe); > + > + if (blkid_do_probe(probe) != 0) { > + VIR_INFO("No filesystem found on device '%s'", device); > + return VIR_STORAGE_BLKID_PROBE_UNDEFINED; > + } > + > + if (blkid_probe_lookup_value(probe, "TYPE", &fstype, NULL) == 0) { > + if (STREQ(fstype, format)) > + return VIR_STORAGE_BLKID_PROBE_MATCH; > + > + return VIR_STORAGE_BLKID_PROBE_DIFFERENT; > + } > + > + if (blkid_known_fstype(format) == 0) > + return VIR_STORAGE_BLKID_PROBE_UNKNOWN; > + > + return VIR_STORAGE_BLKID_PROBE_ERROR; > +} I'm confused. So before this patch set this function was returning something among these lines. Then in patch #2 you make it return -1 or 0, and here we go again.

The code after this patch looks sane though. And I don't really care that much about partial steps. The result looks good, therefore you have my ACK. Michal