On 03/02/2010 09:51 AM, Laine Stump wrote: > On 03/02/2010 05:48 AM, Daniel P. Berrange wrote: >> On Tue, Mar 02, 2010 at 02:17:59AM -0500, Laine Stump wrote: >> >>> + uid_t uid = (vol->target.perms.uid == -1) ? getuid() : >>> vol->target.perms.uid; >>> + gid_t gid = (vol->target.perms.gid == -1) ? getgid() : >>> vol->target.perms.gid; >>> >> This sounds good, but can we encapsulate this conditional logic in a >> helper >> function or macro. eg >> >> uid_t uid = virVolumeDefGetUID(vol) >> >> and so on for later areas of the patch >> > > Sure. That will be putting the getuid/getgid calls back into > storage_conf (.h at least), but it will make things a bit more readable. > > I'll make a new version. > A few of us talked offline, and decided we should commit this patch as is for the release. Dan's proposed change is just a cleanup which can wait, and the patch fixes a regression when building pools with virt-manager, so I've just pushed it. Thanks, Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list