[libvirt] [PATCH v2] qemuDomainSaveMemory: Don't enforce dynamicOwnership

[Michal Privoznik]

On 07/24/2018 10:40 PM, John Ferlan wrote:
> 
> 
> On 07/09/2018 08:51 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1589115
>>
>> When doing a memory snapshot qemuOpenFile() is used. This means
>> that the file where memory is saved is firstly attempted to be
>> created under root:root (because that's what libvirtd is running
>> under) and if this fails the second attempt is done under
>> domain's uid:gid. This does not make much sense - qemu is given
>> opened FD so it does not need to access the file. Moreover, if
>> dynamicOwnership is set in qemu.conf and the file lives on a
>> squashed NFS this is deadly combination and very likely to fail.
>>
>> The fix consists of using:
>>
>>   qemuOpenFileAs(fallback_uid = cfg->user,
>>                  fallback_gid = cfg->group,
>>                  dynamicOwnership = false)
>>
>> In other words, dynamicOwnership is turned off for memory
>> snapshot (chown() will still be attempted if the file does not
>> live on NFS) and instead of using domain DAC label, configured
>> user:group is set as fallback.
>>
> 
> for memory snapshot and core files, right?
> 
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>
>> Diff to v1:
>> - Fix doCoreDump too (raised in review by John).
>>
>>  src/qemu/qemu_driver.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>
> 
> Strange - I had this marked as I replied to it, but obviously I didn't.
> Wonder WTF happened ...
> 
> and the second qemuOpenFile in qemuDomainSaveMemory to touch up the
> header (virQEMUSaveDataFinish) probably could use qemuOpenFileAs too
> right?  although perhaps less important since the answer should be the
> same, just the journey a little different.

Not really. The second time we're opening the file it exists already
(notice we are not passing O_CREAT flag). This means we will not touch
the owner of the file.

> 
> Leaving just one consumer for qemuOpenFile and dynamic_ownership
> manipulation.
> 
> Reviewed-by: John Ferlan <jferlan at redhat.com>
> 

Pushed, thanks.

Michal