[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Add support for shared sanlock leases




----- Original Message -----
> From: "Eric Blake" <eblake redhat com>
> To: "Federico Simoncelli" <fsimonce redhat com>
> Cc: "Daniel P. Berrange" <berrange redhat com>, libvir-list redhat com
> Sent: Friday, June 22, 2012 6:19:34 AM
> Subject: Re: [libvirt] [PATCH] Add support for shared sanlock leases
> 
> On 06/21/2012 11:00 AM, Federico Simoncelli wrote:
> > On Thu, Jun 21, 2012 at 03:37:10PM +0100, Daniel P. Berrange wrote:
> >> From: "Daniel P. Berrange" <berrange redhat com>
> >>
> >> A sanlock lease can be marked as shared (rather
> >> than exclusive)  using SANLK_RES_SHARED flag. This
> >> adds support for that flag and ensures that in auto
> >> disk mode, any shared disks use shared leases. This
> >> also makes any read-only disks be completely
> >> ignored.
> >>
> 
> >>      case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK:
> >>          if (driver->autoDiskLease) {
> >> -            if (virLockManagerSanlockAddDisk(lock, name, nparams,
> >> params) < 0)
> >> +            if (virLockManagerSanlockAddDisk(lock, name, nparams,
> >> params,
> >> +                                             !!(flags &
> >> VIR_LOCK_MANAGER_RESOURCE_SHARED)) < 0)
> > 
> > Just out of curiosity, you are using "!!" (both here and below)
> > because
> > the compiler is complaining about the type?
> 
> Rather, it is a way of forcing a value to be 0 or 1 with the fewest
> characters possible (since the line is already long); shorter than
> constructs such as:
> 
> (bool)(flags & VIR_LOCK_MANAGER_RESOURCE_SHARED)
> (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED) != 0

Yes, it is clear to me that it was a shortcut to squash the values
to 0|1 (as coincidence Daniel few days ago received a patch from me
with a similar trick). It would be nice to know if the compiler is
smart enough to optimize it (instead of doing the double negation).
Beware... not that it would make any difference here :)

> But since virLockManagerSanlockAddDisk() declared its argument as
> bool, and we already require a C99 compiler, the !! trick is strictly
> unnecessary; C99 compilers already guarantee to implicitly convert
> any non-zero value to true when doing argument conversion.

Ah good, this is what I wanted to know. Is the compiler going to
silently convert the value or is it going to produce a warning?
Looks like you're saying that it's the former.

-- 
Federico


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]