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?
Sorry for the late reply. OK, awesome. Will rebase again. Thanks Martin.
- Michael