On 01/09/2017 10:32 AM, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
>> ---
>> 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.
IIRC, the diffs were really ugly with the 1 step approach.
Maybe if I created a step between 1 and 2 which introduced and
implemented the VIR_STORAGE_BLKID_PROBE_* constants that may make it
easier going from 2 to 3.
John
>
> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
