[libvirt] [PATCH 07/14] Implement dispatch functions for lock protocol in virtlockd

[Daniel P. Berrange]

On Wed, Dec 12, 2012 at 07:14:15PM +0100, Michal Privoznik wrote:
> On 11.12.2012 21:41, Daniel P. Berrange wrote:
> > +
> > +static int
> > +virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNUSED,
> > +                                            virNetServerClientPtr client,
> > +                                            virNetMessagePtr msg ATTRIBUTE_UNUSED,
> > +                                            virNetMessageErrorPtr rerr,
> > +                                            virLockSpaceProtocolAcquireResourceArgs *args)
> > +{
> > +    int rv = -1;
> > +    unsigned int flags = args->flags;
> > +    virLockDaemonClientPtr priv =
> > +        virNetServerClientGetPrivateData(client);
> > +    virLockSpacePtr lockspace;
> > +    unsigned int newFlags;
> > +
> > +    virMutexLock(&priv->lock);
> > +
> > +    virCheckFlagsGoto(VIR_LOCK_SPACE_ACQUIRE_SHARED |
> > +                      VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, cleanup);
> 
> se here you are effectively checking 'flags' to
> VIR_LOCK_SPACE_ACQUIRE_SHARED and VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE ...

Yep, obviously the current code is wrong. I'll fix.

> 
> > +
> > +    if (priv->restricted) {
> > +        virReportError(VIR_ERR_OPERATION_DENIED, "%s",
> > +                       _("lock manager connection has been restricted"));
> > +        goto cleanup;
> > +    }
> > +
> > +    if (!priv->ownerPid) {
> > +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> > +                       _("lock owner details have not been registered"));
> > +        goto cleanup;
> > +    }
> > +
> > +    if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Lockspace for path %s does not exist"),
> > +                       args->path);
> > +        goto cleanup;
> > +    }
> > +
> > +    newFlags = 0;
> > +    if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED)
> > +        newFlags |= VIR_LOCK_SPACE_ACQUIRE_SHARED;
> > +    if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE)
> > +        newFlags |= VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE;
> 
> while here you test them to a different set of flags. I understand you
> want to do translation from protocol representation of flags to API one
> since they may (but shouldn't) diverge. You need to update the
> virCheckFlagsGoto() then.
> 
> > +
> > +    if (virLockSpaceAcquireResource(lockspace,
> > +                                    args->name,
> > +                                    priv->ownerPid,
> > +                                    newFlags) < 0) {
> > +        VIR_ERROR("FAILED");
> > +        goto cleanup;
> > +    }
> > +
> > +        VIR_ERROR("OK");
> 
> indentation

Those two VIR_ERROR lines will just be killed - they were random debug.

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 :|