On Wed, Mar 19, 2025 at 05:43:20PM -0600, Jim Fehlig via Devel wrote:
> On 3/19/25 05:54, Daniel P. Berrangé wrote:
> > On Wed, Mar 05, 2025 at 03:48:10PM -0700, Jim Fehlig via Devel wrote:
> > > When invoking virDomainSaveParams with a relative path, the image is
> > > saved to the daemon's CWD. Similarly, when providing
virDomainRestoreParams
> > > with a relative path, it attempts to restore from the daemon's CWD. In
most
> > > configurations, the daemon's CWD is set to '/'. Ensure a relative path is
> > > converted to absolute before invoking the driver domain{Save,Restore}
Params
> > > functions.
> >
> > Are you aware of any common usage of these APIs with a relative path ?
>
> No. I added this patch to the series after receiving feedback from testers
that
> included "Providing a relative path to the location of the saved VM does not
> work". E.g. something like
>
> # virsh restore --bypass-cache test/test-vm.sav
> error: Failed to restore domain from test/test-vm.sav
> error: Failed to open file 'test/test-vm.sav': No such file or directory"
>
> > Although we've not documented it, my general view is that all paths given
> > to libvirt are expected to be absolute, everywhere - whether API parameters
> > like these, or config file parmeters, or XML elements/attributes.
>
> Agreed. Relative paths from (remote) clients are ambiguous on the server. I'm
> fine dropping this patch from the series.
>
> > In the perfect world we would canonicalize all relative paths on the
> > client side, but doing that is such an enourmous & complex job it is
> > not likely to be practical. We'd be better off just documenting use
> > of relative paths as "out of scope" and / or "undefined behaviour"
>
> I'll take a stab at improving the documentation.
>
So this actually breaks an existing usage, albeit from a simple user
(e.g. myself). And because later patches switch from
virDomain{Save,Restore}Flags to virDomain{Save,Restore}Params, this
might be seen as a regression. If we do not want to canonicalize the
paths, then we should error out on relative ones. Without that the
current behaviour is not only what's described above (and how I noticed
it), but also the following two commands:
virsh save domain asdf.img
virsh save --image-format raw asdf.img
will behave differently. The first one saves the image in CWD of virsh,
the second one will save the same image in CWD of virtqemud (or
respective daemon), without any error or indication of that happening.
Consequently, `virsh restore` will restore from a different file based
on whether there are extra flags/parameters or not.
