[libvirt] [PATCH 2/3] qemuDomainGetPreservedMounts: Prune nested mount points

[Michal Privoznik]

On 06/14/2017 09:52 PM, John Ferlan wrote:
> 
> 
> On 06/12/2017 11:57 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1431112
>>
>> There can be nested mount points. For instance /dev/shm/blah can
>> be a mount point and /dev/shm too. It doesn't make much sense to
>> return the former path because callers preserve the latter (and
>> with that the former too). Therefore prune nested mount points.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/qemu/qemu_domain.c | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 23b92606e..accf05a6f 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -7533,7 +7533,7 @@ qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
>>                               size_t *ndevPath)
>>  {
>>      char **paths = NULL, **mounts = NULL;
>> -    size_t i, nmounts;
>> +    size_t i, j, nmounts;
>>  
>>      if (virFileGetMountSubtree(PROC_MOUNTS, "/dev",
>>                                 &mounts, &nmounts) < 0)
>> @@ -7545,6 +7545,27 @@ qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg,
>>          return 0;
>>      }
>>  
>> +    /* There can be nested mount points. For instance
>> +     * /dev/shm/blah can be a mount point and /dev/shm too. It
>> +     * doesn't make much sense to return the former path because
>> +     * caller preserves the latter (and with that the former
>> +     * too). Therefore prune nested mount points.
>> +     * NB mounts[0] is "/dev". Should we start the outer loop
>> +     * from the beginning of the array all we'd be left with is
>> +     * just the first element. Think about it.
>> +     */
>> +    for (i = 1; i < nmounts; i++) {
>> +        for (j = i + 1; j < nmounts;) {
>> +            if (STRPREFIX(mounts[j], mounts[i])) {
>> +                VIR_DEBUG("Dropping path %s because of %s", mounts[j], mounts[i]);
>> +                VIR_DELETE_ELEMENT(mounts, j, nmounts);
>> +            } else {
>> +                j++;
>> +            }
> 
> Ewww
> 
> I prefer a :
> 
>    j = i + 1;
>    while (j < nmounts) {
>       if ()
>          ...
>       else
>          j++;
>    }

Okay. I prefer the other approach (what I have here), but I don't care
that much.

Michal