[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] esx: Fix regression in absolute file name handling



2011/5/26 Eric Blake <eblake redhat com>:
> On 05/26/2011 09:45 AM, Matthias Bolte wrote:
>>
>> -        if (datastorePath == NULL) {
>> +        /* If it's an absolute path outside of a datastore just use it as is */
>> +        if (result == NULL && *fileName == '/') {
>> +            /* FIXME: need to deal with Windows paths here too */
>
> #include "dosname.h" // from gnulib, already imported into libvirt
>
> if (result == NULL && IS_ABSOLUTE_FILE_NAME(fileName)) {
>
> and that will properly detect '/' on Unix and 'c:\' on Windows paths
> according to what OS that you compiled libvirt to run on.  I am assuming
> that the absolute paths in question are residing on the machine where
> libvirt is running.
>
> (I'm not familiar enough with esx, so my assumption might be wrong, with
> this code being a generic situation of trying to recognize a string as
> an absolute path from the point of the view of the machine where the
> guest is running, even though libvirt is running on a different system
> and will never be able to actually probe if such a file exists.  If so,
> then you have to do the parsing work yourself, since
> IS_ABSOLUTE_FILE_NAME compiled on Linux won't recognize Windows-style
> names as absolute - it only recognizes windows names on mingw or cygwin
> - but at least you have dosname.h to give some hints on how to do the
> parsing).

libvirt is not running the the ESX/GSX server and the file names are
local to the ESX/GSX server. Therefore, IS_ABSOLUTE_FILE_NAME won't
work here.

>> +
>> +        result = virBufferContentAndReset(&buffer);
>> +    } else if (*fileName == '/') {
>> +        /* FIXME: need to deal with Windows paths here too */
>
> And again.
>
> But I don't see anything that would prevent using this patch as-is,
> leaving absolute windows names for a subsequent patch; and since it
> fixes a regression:
>
> ACK.
>

Thanks, pushing this patch as is.

Matthias


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]