On Tue, Oct 13, 2015 at 15:11:01 -0400, John Ferlan wrote:
On 10/13/2015 08:50 AM, Peter Krempa wrote:
> On Fri, Oct 09, 2015 at 09:34:07 -0400, John Ferlan wrote:
...
>
> 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.
It might be a better option to refactor it prior to tweaking the cleanup
path.
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?
Perhaps even without the netfs check if you think it makes sense so that
the logic isn't overcomplicated.
Peter