On 10/13/2015 08:50 AM, Peter Krempa wrote:
> On Fri, Oct 09, 2015 at 09:34:07 -0400, John Ferlan wrote:
>> After a successful qemu-img/qcow-create of the backing file, if we
>> fail to stat the file, change it owner/group, or mode, then the
>> cleanup path should delete the file.
>>
>> Also moved the virCommandSetUID/virCommandSetGID inside the condition
>> used to actually run the command rather than randomly setting and not
>> using it if the file had been created. The 'cmd' buffer is only used
>> if we need to create.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/storage/storage_backend.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>> index a375fe0..7d0de63 100644
>> --- a/src/storage/storage_backend.c
>> +++ b/src/storage/storage_backend.c
>
> ...
>
>> @@ -737,6 +740,8 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
>> ret = 0;
>>
>> cleanup:
>> + if (ret < 0 && filecreated)
>> + unlink(vol->target.path);
>
> This might not work if the volume was created with different uid/gid as
> the process that is attempting to delete it (in case of e.g. NFS.).
>
Ugh... this function was really strange especially that chown/chmod
after a create on an NETFS target. The net result if the first pile
of 'ifs' passes is a creation in a NETFS pool using either 'qemu-img'
or 'qcow-create' under the uid/gid from the vol->target.{uid|gid}.
So yes, we'd have to virFileRemove that.
The other creation would able to unlink directly, although I suppose a
revector into virFileRemove would work, although it'd be passing uid,
gid == -1, -1.
I know you don't necessarily prefer inline diffs, but this one's fairly
short:
- if (ret < 0 && filecreated)
- unlink(vol->target.path);
+ if (ret < 0 && filecreated) {
+ if (netfs)
+ virFileDelete(vol->target.path, vol->target.uid, vol->target.gid);
+ else
+ unlink(vol->target.path);
+ }
where 'netfs' is a new bool set when we create in that first if
Does this look more reasonable?
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list