On 6/25/21 4:53 AM, Michal Prívozník wrote:
On 5/28/21 8:30 PM, Jim Fehlig wrote:
> Hi All!
>
> I received a bug report about virtlockd emitting an error whenever
> libvirtd is (re)started
>
> May 25 15:44:31 virt81 virtlockd[7723]: Requested operation is not
> valid: Lockspace for path /data/libvirtd/lockspace already exists
>
> The problem is easily reproducible with git master by enabling lockd in
> qemu.conf, setting file_lockspace_dir in qemu-lockd.conf, then
> restarting libvirtd.
>
> If I understand the code correctly, when the qemu driver loads, it calls
> virLockManagerPluginNew, which dlopens lockd.so and calls drvInit, aka
> virLockManagerLockDaemonInit. Here the driver object is created, config
> loaded, and virLockManagerLockDaemonSetupLockspace is called.
> virLockManagerLockDaemonSetupLockspace sends
> virLockSpaceProtocolCreateLockSpaceArgs rpc to virtlockd, where it is
> dispatched to virLockSpaceProtocolDispatchCreateLockSpace. Alas we
> encounter the error when virLockDaemonFindLockSpace finds the existing
> lockspace.
>
> I'm not really sure how to go about fixing it and fishing for opinions.
> virLockManagerLockDaemonSetupLockspace already has some code to handle
> the error
>
>
https://gitlab.com/libvirt/libvirt/-/blob/master/src/locking/lock_driver_...
>
>
> Since libvirtd ignores VIR_ERR_OPERATION_INVALID, should virtlockd be
> changed to not return error in that case? It would be better if libvirtd
> knew it already told virtlockd to configure the lockspace and avoid
> needlessly doing it again.
Yeah, that would be the best. But I'm not sure we have a reliable way to
store that info. I mean, what if I'd shut down libvirtd, then virtlockd
and then 'rm -rf $lockDir'.
Fortunately, the RPC between virtlockd and libvirtd is considered
internal enough that we can change it. We have guaranteed by the
specfile that the virtlockd and libvirtd will be the same version.
Therefore, I think we can add a new flag "hey, it's okay if the
lockspace exists, no need to report error".
I explored this option (see attached patch) but when testing realized whatever
is specified in file_lockspace_dir must be created by the user anyhow. If not,
VMs fail to start
# virsh start test
error: Failed to start domain 'test'
error: Unable to open/create resource
/data/libvirtd/lockspace/de22c4bf931e7c48b49e8ca64b477d44e78a51543e534df488b05ccd08ec5caa:
No such file or directory
Since the user must create the lockspace, it will always exist :-).
Maybe we can change virtlockd to not report error if the lockspace
exists even without the flag.
Callers are not interested in the error, so I took this approach
https://listman.redhat.com/archives/libvir-list/2021-June/msg00841.html
Regards,
Jim