On Sun, Sep 08, 2024 at 08:00:02AM +0200, Martin Kletzander wrote:
On Thu, Aug 15, 2024 at 01:30:38PM -0500, Michael Galaxy wrote:On 8/7/24 10:10, Michael Galaxy wrote:On 8/7/24 08:23, Martin Kletzander wrote:Exactly, we do not want the paths to change. But if you start the VM, then stop virtqemud/libvirtd (totally supported), change the config file to point to a different directory (or even different number of directories, I haven't even thought of that), then start the daemon back again, we'll have a problem when the VM is being cleaned up. For these kinds of situations we are keeping more pieces of information about running domains in a so called status XML. This is saved on disk, but unavailable to the user so saving something there does not explicitly expose it to the user. Having the paths saved there would make it nicer to clean up and it should've probably been done even with that one path that is supported nowadays. I can have a look at fixing that at first and then your patches could be applied on top of that fix if that's fine with you or you can have a look at tackling that yourself, it should not be difficult, tests for that should be nice to write as well, it just needs to be done in a back-compat way, of course. See qemuDomainObjPrivateXMLFormat() and qemuDomainObjPrivateXMLParse() for some starting points.I've finished addressing most of the rest of the review comments here and I'll sent that out, but I'd like to avoid too many more revisions here. IMHO, the problem above is really orthogonal to this feature and, as you said, applies to the original (single path) case, so I'd like to leave that as a homework assignment to the community.Yes, but once we start changing things it is harder to fix them.Realistically, in a well-managed cloud, this is highly unlikely to happen, so I'm not too worried about it.I am not talking about accidental changes. It is fine until someone "really needs" to change that value and expects libvirt to behave correctly.Would a ticket be helpful somewhere? I don't know if libvirt has a way of tracking these kinds of discoveries, I'd be happy to open a ticket somwhere and put it in the libvirt backlog.Sure, you can file an issue on gitlab: https://gitlab.com/libvirt/libvirt/-/issues/new but I'll try to work on and post a quick fix anyway so that it does not block my already late review of your v3.
Sorry, I completely forgot to let you know that the fix is already in the latest release, last commit of the series in master is 6f0974ca32ce. Would you mind rebasing your patches on top of current master to reflect the changes? Thank you, Martin
- Michael
Attachment:
signature.asc
Description: PGP signature
