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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list