On Fri, Apr 17, 2015 at 03:36:20PM -0600, Jim Fehlig wrote:
> Some hypervisors like Xen do not have PIDs associated with domains.
> Relax the requirement for PID != 0 in the locking code so it can
> be used by hypervisors that do not represent domains as a process
> running on the host.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> src/locking/lock_daemon.c | 2 +-
> src/locking/lock_daemon_dispatch.c | 49 +++++++++++---------------------------
> src/locking/lock_driver_lockd.c | 7 ++----
> 3 files changed, 17 insertions(+), 41 deletions(-)
>
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
> index bb165c0..042ff94 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -678,7 +678,7 @@ virLockDaemonClientFree(void *opaque)
> signum = SIGKILL;
> else
> signum = 0;
> - if (virProcessKill(priv->clientPid, signum) < 0) {
> + if (priv->clientPid != 0 && virProcessKill(priv->clientPid, signum) < 0) {
> if (errno == ESRCH)
> break;
>
> diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c
> index a7cee9d..f2704ec 100644
> --- a/src/locking/lock_daemon_dispatch.c
> +++ b/src/locking/lock_daemon_dispatch.c
> @@ -62,11 +62,8 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNU
> goto cleanup;
> }
>
> - if (!priv->ownerPid) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("lock owner details have not been registered"));
> - goto cleanup;
> - }
> + if (!priv->ownerPid)
> + VIR_WARN("lock owner PID has not been registered");
In this file, all these ownerPid checks are really there to validate that
the virLockSpaceProtocolDispatchRegister() method has been called first.
I think we need to check that check, so rather than removing all these
error reports, lets change them all to check priv->ownerId instead of
priv->ownerPid.
>
> if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -120,11 +117,8 @@ virLockSpaceProtocolDispatchCreateResource(virNetServerPtr server ATTRIBUTE_UNUS
> goto cleanup;
> }
>
> - if (!priv->ownerPid) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("lock owner details have not been registered"));
> - goto cleanup;
> - }
> + if (!priv->ownerPid)
> + VIR_WARN("lock owner PID has not been registered");
>
> if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -169,11 +163,8 @@ virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr server ATTRIBUTE_UNUS
> goto cleanup;
> }
>
> - if (!priv->ownerPid) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("lock owner details have not been registered"));
> - goto cleanup;
> - }
> + if (!priv->ownerPid)
> + VIR_WARN("lock owner PID has not been registered");
>
> if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -218,11 +209,8 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server ATTRIBUTE_UNUSED,
> goto cleanup;
> }
>
> - if (!priv->ownerPid) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("lock owner details have not been registered"));
> - goto cleanup;
> - }
> + if (!priv->ownerPid)
> + VIR_WARN("lock owner PID has not been registered");
>
> if (!args->path || STREQ(args->path, "")) {
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> @@ -273,11 +261,8 @@ virLockSpaceProtocolDispatchRegister(virNetServerPtr server ATTRIBUTE_UNUSED,
> goto cleanup;
> }
>
> - if (priv->ownerPid) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("lock owner details have already been registered"));
> - goto cleanup;
> - }
> + if (priv->ownerPid)
> + VIR_WARN("lock owner PID has not been registered");
>
> if (VIR_STRDUP(priv->ownerName, args->owner.name) < 0)
> goto cleanup;
> @@ -320,11 +305,8 @@ virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr server ATTRIBUTE_UNU
> goto cleanup;
> }
>
> - if (!priv->ownerPid) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("lock owner details have not been registered"));
> - goto cleanup;
> - }
> + if (!priv->ownerPid)
> + VIR_WARN("lock owner PID has not been registered");
>
> if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -370,11 +352,8 @@ virLockSpaceProtocolDispatchRestrict(virNetServerPtr server ATTRIBUTE_UNUSED,
> goto cleanup;
> }
>
> - if (!priv->ownerPid) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("lock owner details have not been registered"));
> - goto cleanup;
> - }
> + if (!priv->ownerPid)
> + VIR_WARN("lock owner PID has not been registered");
>
> priv->restricted = true;
> rv = 0;
> diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
> index 3a48a6a..109a79b 100644
> --- a/src/locking/lock_driver_lockd.c
> +++ b/src/locking/lock_driver_lockd.c
> @@ -466,11 +466,8 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock,
> _("Missing ID parameter for domain object"));
> return -1;
> }
> - if (priv->pid == 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Missing PID parameter for domain object"));
> - return -1;
> - }
> + if (priv->pid == 0)
> + VIR_WARN("Missing PID parameter for domain object");
Lets just make that a VIR_DEBUG, since you don't want the log polluted
with warnings every time a xen guest is started.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list