[libvirt] [PATCH 2/2] qemu: don't log error for missing optional sources on start

[Daniel P. Berrangé]

On Tue, Dec 11, 2018 at 09:33:50AM -0500, John Ferlan wrote:
> 
> 
> On 12/11/18 2:39 AM, Nikolay Shirokovskiy wrote:
> > 
> > 
> > On 11.12.2018 01:05, John Ferlan wrote:
> >> $SUBJ
> >>
> >> "storage sources"
> >>
> >> On 11/12/18 7:58 AM, Nikolay Shirokovskiy wrote:
> >>> Because missing optional source is not error. The patch
> >>> address only local files. Fixing other cases is a bit ugly.
> >>>
> >>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> >>> ---
> >>>  src/qemu/qemu_process.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> >>> index 802274e..5e04bf9 100644
> >>> --- a/src/qemu/qemu_process.c
> >>> +++ b/src/qemu/qemu_process.c
> >>> @@ -6100,7 +6100,8 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
> >>>          if (!blockdev)
> >>>              virStorageSourceBackingStoreClear(disk->src);
> >>>  
> >>> -        if (qemuDomainDetermineDiskChain(driver, vm, disk, true) >= 0)
> >>> +        if (!qemuProcessMissingLocalOptionalDisk(disk) &&
> >>> +            qemuDomainDetermineDiskChain(driver, vm, disk, true) >= 0)
> >>
> >> Although it makes more sense for this path to use the startupPolicy, I
> >> will point out that the first thing qemuDomainDetermineDiskChain does is
> >> filter on virStorageSourceIsEmpty, so regardless of whether the
> >> startupPolicy is optional or not, not much is happening for empty
> >> sources.  So in essence unnecessary at least from my read.
> > 
> > The issue is not with empty (no source file is specified) sources but rather
> > with missing ones (source file is specified but file itself does not exist).
> > In this case virStorageSourceIsEmpty does not help and we get this log notice:
> > 
> > error: virStorageFileReportBrokenChain:427 : Cannot access storage file '/path/to/missing/optional/disk': No such file or directory
> > 
> > Which is not fatal for starting a domain because source is optional.
> 
> But, but not logging it how would someone know that it's missing
> otherwise?  Maybe seeing that message in a log file draws someone's
> attention to the fact that their source went missing and even though it
> is optional, they may be surprised that the file isn't there. This is
> one of those hard one's where it's difficult to read the minds of all
> consumers. Damned by some if you do log and damned by others if you
> don't log.

IMHO if you have marked the source as optional, you have explicitly
stated that you don't care if it goes missing for *any* reason. As
such we certainly should not log anything at "error" or "warning"
levels. At the very most it would be an "info" level warning if at
all.

If apps are concerned about things going missing at the wrong time,
they should look at the XML once the guest is running to see if the
expected source is there, or validate it themselves before booting
the guest.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|