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.
Either we need to disallow relative paths or canonicalize them on the
client, and ideally before the release.
Regards, Jim
signature.asc
Description: PGP signature
