All of lore.kernel.org
 help / color / mirror / Atom feed
* libuuid vs boost uuid
@ 2013-11-09  5:59 James Harper
  2013-11-09  6:43 ` Sage Weil
  0 siblings, 1 reply; 14+ messages in thread
From: James Harper @ 2013-11-09  5:59 UTC (permalink / raw)
  To: ceph-devel

Just out of curiosity (recent thread about windows port) I just had a quick go at compiling librados under mingw (win32 cross compile), and one of the errors that popped up was the lack of libuuid under mingw. Ceph appears to use libuuid, but I notice boost appears to include a uuid class too, and it seems that ceph already uses some of boost (which already builds under mingw).

Is there anything special about libuuid that would mean boost's uuid class couldn't replace it? And would it be better to still use ceph's uuid.h as a wrapper around the boost uuid class, or to modify ceph to use the boost uuid class directly?

Thanks

James

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-09  5:59 libuuid vs boost uuid James Harper
@ 2013-11-09  6:43 ` Sage Weil
  2013-11-09 16:41   ` Noah Watkins
  2013-11-10  1:44   ` James Harper
  0 siblings, 2 replies; 14+ messages in thread
From: Sage Weil @ 2013-11-09  6:43 UTC (permalink / raw)
  To: James Harper; +Cc: ceph-devel

On Sat, 9 Nov 2013, James Harper wrote:
> Just out of curiosity (recent thread about windows port) I just had a 
> quick go at compiling librados under mingw (win32 cross compile), and 
> one of the errors that popped up was the lack of libuuid under mingw. 
> Ceph appears to use libuuid, but I notice boost appears to include a 
> uuid class too, and it seems that ceph already uses some of boost (which 
> already builds under mingw).
> 
> Is there anything special about libuuid that would mean boost's uuid 
> class couldn't replace it? And would it be better to still use ceph's 
> uuid.h as a wrapper around the boost uuid class, or to modify ceph to 
> use the boost uuid class directly?

Nice!  Boost uuid looks like it would work just fine.  It is probably 
easier and less disruptive to use it from within the existing class in 
include/uuid.h.

sage

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-09  6:43 ` Sage Weil
@ 2013-11-09 16:41   ` Noah Watkins
  2013-11-09 17:25     ` asomers
  2013-11-10  1:44   ` James Harper
  1 sibling, 1 reply; 14+ messages in thread
From: Noah Watkins @ 2013-11-09 16:41 UTC (permalink / raw)
  To: Sage Weil; +Cc: James Harper, ceph-devel, Alan Somers

Alan, would this fix the problem on FreeBSD? IIRC llibuuid is a
terrible terrible headache, with the recommended approach, being to
upstream changes to e2fsprogs-libuuid.

On Fri, Nov 8, 2013 at 10:43 PM, Sage Weil <sage@inktank.com> wrote:
> On Sat, 9 Nov 2013, James Harper wrote:
>> Just out of curiosity (recent thread about windows port) I just had a
>> quick go at compiling librados under mingw (win32 cross compile), and
>> one of the errors that popped up was the lack of libuuid under mingw.
>> Ceph appears to use libuuid, but I notice boost appears to include a
>> uuid class too, and it seems that ceph already uses some of boost (which
>> already builds under mingw).
>>
>> Is there anything special about libuuid that would mean boost's uuid
>> class couldn't replace it? And would it be better to still use ceph's
>> uuid.h as a wrapper around the boost uuid class, or to modify ceph to
>> use the boost uuid class directly?
>
> Nice!  Boost uuid looks like it would work just fine.  It is probably
> easier and less disruptive to use it from within the existing class in
> include/uuid.h.
>
> sage
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-09 16:41   ` Noah Watkins
@ 2013-11-09 17:25     ` asomers
  0 siblings, 0 replies; 14+ messages in thread
From: asomers @ 2013-11-09 17:25 UTC (permalink / raw)
  To: Noah Watkins; +Cc: Sage Weil, James Harper, ceph-devel

It certainly would.  For those not in the loop, e2fsprogs-libuuid is
problematic on FreeBSD because it shadows a symbol defined in libc.
There's a workaround, but boost::uuid would be better.
-Alan

On Sat, Nov 9, 2013 at 9:41 AM, Noah Watkins <noah.watkins@inktank.com> wrote:
> Alan, would this fix the problem on FreeBSD? IIRC llibuuid is a
> terrible terrible headache, with the recommended approach, being to
> upstream changes to e2fsprogs-libuuid.
>
> On Fri, Nov 8, 2013 at 10:43 PM, Sage Weil <sage@inktank.com> wrote:
>> On Sat, 9 Nov 2013, James Harper wrote:
>>> Just out of curiosity (recent thread about windows port) I just had a
>>> quick go at compiling librados under mingw (win32 cross compile), and
>>> one of the errors that popped up was the lack of libuuid under mingw.
>>> Ceph appears to use libuuid, but I notice boost appears to include a
>>> uuid class too, and it seems that ceph already uses some of boost (which
>>> already builds under mingw).
>>>
>>> Is there anything special about libuuid that would mean boost's uuid
>>> class couldn't replace it? And would it be better to still use ceph's
>>> uuid.h as a wrapper around the boost uuid class, or to modify ceph to
>>> use the boost uuid class directly?
>>
>> Nice!  Boost uuid looks like it would work just fine.  It is probably
>> easier and less disruptive to use it from within the existing class in
>> include/uuid.h.
>>
>> sage
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: libuuid vs boost uuid
  2013-11-09  6:43 ` Sage Weil
  2013-11-09 16:41   ` Noah Watkins
@ 2013-11-10  1:44   ` James Harper
  2013-11-10  5:22     ` Sage Weil
  1 sibling, 1 reply; 14+ messages in thread
From: James Harper @ 2013-11-10  1:44 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel

> 
> On Sat, 9 Nov 2013, James Harper wrote:
> > Just out of curiosity (recent thread about windows port) I just had a
> > quick go at compiling librados under mingw (win32 cross compile), and
> > one of the errors that popped up was the lack of libuuid under mingw.
> > Ceph appears to use libuuid, but I notice boost appears to include a
> > uuid class too, and it seems that ceph already uses some of boost (which
> > already builds under mingw).
> >
> > Is there anything special about libuuid that would mean boost's uuid
> > class couldn't replace it? And would it be better to still use ceph's
> > uuid.h as a wrapper around the boost uuid class, or to modify ceph to
> > use the boost uuid class directly?
> 
> Nice!  Boost uuid looks like it would work just fine.  It is probably
> easier and less disruptive to use it from within the existing class in
> include/uuid.h.
> 

That seems to work (the header compiles at least), but then it falls down when things try to memcpy out of it. In particular, an fsid appears to be a char[16]. Is that a uuid? And is keeping it as a byte array an optimisation?

James

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: libuuid vs boost uuid
  2013-11-10  1:44   ` James Harper
@ 2013-11-10  5:22     ` Sage Weil
  2013-11-13 15:07       ` Noah Watkins
  0 siblings, 1 reply; 14+ messages in thread
From: Sage Weil @ 2013-11-10  5:22 UTC (permalink / raw)
  To: James Harper; +Cc: ceph-devel

On Sun, 10 Nov 2013, James Harper wrote:
> > 
> > On Sat, 9 Nov 2013, James Harper wrote:
> > > Just out of curiosity (recent thread about windows port) I just had a
> > > quick go at compiling librados under mingw (win32 cross compile), and
> > > one of the errors that popped up was the lack of libuuid under mingw.
> > > Ceph appears to use libuuid, but I notice boost appears to include a
> > > uuid class too, and it seems that ceph already uses some of boost (which
> > > already builds under mingw).
> > >
> > > Is there anything special about libuuid that would mean boost's uuid
> > > class couldn't replace it? And would it be better to still use ceph's
> > > uuid.h as a wrapper around the boost uuid class, or to modify ceph to
> > > use the boost uuid class directly?
> > 
> > Nice!  Boost uuid looks like it would work just fine.  It is probably
> > easier and less disruptive to use it from within the existing class in
> > include/uuid.h.
> > 
> 
> That seems to work (the header compiles at least), but then it falls 
> down when things try to memcpy out of it. In particular, an fsid appears 
> to be a char[16]. Is that a uuid? And is keeping it as a byte array an 
> optimisation?

Probably just being lazy; where was that?  Feel free to replace the memcpy 
with methods to copy in/out if it's necessary...

sage

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-10  5:22     ` Sage Weil
@ 2013-11-13 15:07       ` Noah Watkins
  2013-11-13 21:42         ` James Harper
  0 siblings, 1 reply; 14+ messages in thread
From: Noah Watkins @ 2013-11-13 15:07 UTC (permalink / raw)
  To: Sage Weil; +Cc: James Harper, ceph-devel

Hi James,

I just wanted to follow up on this thread. I’d like to bring this patch into the wip-port portability branch. Were you able to get the boost::uuid to work as a drop-in replacement?

Thanks,
Noah

On Nov 9, 2013, at 9:22 PM, Sage Weil <sage@inktank.com> wrote:

> On Sun, 10 Nov 2013, James Harper wrote:
>>> 
>>> On Sat, 9 Nov 2013, James Harper wrote:
>>>> Just out of curiosity (recent thread about windows port) I just had a
>>>> quick go at compiling librados under mingw (win32 cross compile), and
>>>> one of the errors that popped up was the lack of libuuid under mingw.
>>>> Ceph appears to use libuuid, but I notice boost appears to include a
>>>> uuid class too, and it seems that ceph already uses some of boost (which
>>>> already builds under mingw).
>>>> 
>>>> Is there anything special about libuuid that would mean boost's uuid
>>>> class couldn't replace it? And would it be better to still use ceph's
>>>> uuid.h as a wrapper around the boost uuid class, or to modify ceph to
>>>> use the boost uuid class directly?
>>> 
>>> Nice!  Boost uuid looks like it would work just fine.  It is probably
>>> easier and less disruptive to use it from within the existing class in
>>> include/uuid.h.
>>> 
>> 
>> That seems to work (the header compiles at least), but then it falls 
>> down when things try to memcpy out of it. In particular, an fsid appears 
>> to be a char[16]. Is that a uuid? And is keeping it as a byte array an 
>> optimisation?
> 
> Probably just being lazy; where was that?  Feel free to replace the memcpy 
> with methods to copy in/out if it's necessary...
> 
> sage
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: libuuid vs boost uuid
  2013-11-13 15:07       ` Noah Watkins
@ 2013-11-13 21:42         ` James Harper
  2013-11-13 22:03           ` Noah Watkins
  0 siblings, 1 reply; 14+ messages in thread
From: James Harper @ 2013-11-13 21:42 UTC (permalink / raw)
  To: Noah Watkins, Sage Weil; +Cc: ceph-devel

> Hi James,
> 
> I just wanted to follow up on this thread. I'd like to bring this patch into the
> wip-port portability branch. Were you able to get the boost::uuid to work as
> a drop-in replacement?
> 

I have it compiling but haven't tested. I'll send through what I have.

James

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-13 21:42         ` James Harper
@ 2013-11-13 22:03           ` Noah Watkins
  2013-11-13 22:33             ` James Harper
  0 siblings, 1 reply; 14+ messages in thread
From: Noah Watkins @ 2013-11-13 22:03 UTC (permalink / raw)
  To: James Harper; +Cc: Sage Weil, ceph-devel

Oh ok, no rush.  Just wanted to know if you were still hacking on it. Thanks!

On Nov 13, 2013, at 1:42 PM, James Harper <james.harper@bendigoit.com.au> wrote:

>> Hi James,
>> 
>> I just wanted to follow up on this thread. I'd like to bring this patch into the
>> wip-port portability branch. Were you able to get the boost::uuid to work as
>> a drop-in replacement?
>> 
> 
> I have it compiling but haven't tested. I'll send through what I have.
> 
> James


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: libuuid vs boost uuid
  2013-11-13 22:03           ` Noah Watkins
@ 2013-11-13 22:33             ` James Harper
  2013-11-26  3:46               ` Noah Watkins
  0 siblings, 1 reply; 14+ messages in thread
From: James Harper @ 2013-11-13 22:33 UTC (permalink / raw)
  To: Noah Watkins; +Cc: Sage Weil, ceph-devel

Patch follows. When I wrote it I was just thinking it would be used for win32 build, hence the #ifdef. As I said before, it compiles but I haven't tested it. I can clean it up a bit and resend it with a signed-off-by if anyone wants to pick it up and follow it through sooner than I can. I don't know how boost behaves if the uuid parse fails (exception maybe?) so that would need resolving too.

In addition, a bunch of ceph files include the libuuid header directly, even though all the ones I've found don't appear to need it, so they need to be fixed for a clean compile under win32, and to remove dependency on libuuid. There may also be other cases that need work, in particular anything that memcpy's into the 16 byte uuid directly. See patch for MStatfsReply.h where a minor tweak was necessary.

(if anyone is interested, I have librados and librbd compiling under mingw32, but I can't get boost to build its thread library so I don't get a clean link, and there are probably other link errors too. I've run out of time for doing much more on this for the moment)

James

diff --git a/src/include/uuid.h b/src/include/uuid.h
index 942b807..201ac76 100644
--- a/src/include/uuid.h
+++ b/src/include/uuid.h
@@ -8,6 +8,70 @@
 #include "encoding.h"
 #include <ostream>

+#if defined(_WIN32)
+
+#include <boost/lexical_cast.hpp>
+#include <boost/uuid/uuid.hpp>
+#include <boost/uuid/uuid_generators.hpp>
+#include <boost/uuid/uuid_io.hpp>
+#include <string>
+
+struct uuid_d {
+  boost::uuids::uuid uuid;
+
+  uuid_d() {
+    uuid = boost::uuids::nil_uuid();
+  }
+
+  bool is_zero() const {
+    return uuid.is_nil();
+    //return boost::uuids::uuid::is_nil(uuid);
+  }
+
+  void generate_random() {
+    boost::uuids::random_generator gen;
+    uuid = gen();
+  }
+
+  bool parse(const char *s) {
+    boost::uuids::string_generator gen;
+    uuid = gen(s);
+    return true;
+    // what happens if parse fails?
+  }
+  void print(char *s) {
+    std::string str = boost::lexical_cast<std::string>(uuid);
+    memcpy(s, str.c_str(), 37);
+  }
+
+  void encode(bufferlist& bl) const {
+    ::encode_raw(uuid, bl);
+  }
+  void decode(bufferlist::iterator& p) const {
+    ::decode_raw(uuid, p);
+  }
+
+  uuid_d& operator=(const uuid_d& r) {
+    uuid = r.uuid;
+    return *this;
+  }
+};
+WRITE_CLASS_ENCODER(uuid_d)
+
+inline std::ostream& operator<<(std::ostream& out, const uuid_d& u) {
+  //char b[37];
diff --git a/src/include/uuid.h b/src/include/uuid.h
index 942b807..201ac76 100644
--- a/src/include/uuid.h
+++ b/src/include/uuid.h
@@ -8,6 +8,70 @@
 #include "encoding.h"
 #include <ostream>

+#if defined(_WIN32)
+
+#include <boost/lexical_cast.hpp>
+#include <boost/uuid/uuid.hpp>
+#include <boost/uuid/uuid_generators.hpp>
+#include <boost/uuid/uuid_io.hpp>
+#include <string>
+
+struct uuid_d {
+  boost::uuids::uuid uuid;
+
+  uuid_d() {
+    uuid = boost::uuids::nil_uuid();
+  }
+
+  bool is_zero() const {
+    return uuid.is_nil();
+    //return boost::uuids::uuid::is_nil(uuid);
+  }
+
+  void generate_random() {
+    boost::uuids::random_generator gen;
+    uuid = gen();
+  }
+
+  bool parse(const char *s) {
+    boost::uuids::string_generator gen;
+    uuid = gen(s);
+    return true;
+    // what happens if parse fails?
+  }
+  void print(char *s) {
+    std::string str = boost::lexical_cast<std::string>(uuid);
+    memcpy(s, str.c_str(), 37);
+  }
+
+  void encode(bufferlist& bl) const {
+    ::encode_raw(uuid, bl);
+  }
+  void decode(bufferlist::iterator& p) const {
+    ::decode_raw(uuid, p);
+  }
+
+  uuid_d& operator=(const uuid_d& r) {
+    uuid = r.uuid;
+    return *this;
+  }
+};
+WRITE_CLASS_ENCODER(uuid_d)
+
+inline std::ostream& operator<<(std::ostream& out, const uuid_d& u) {
+  //char b[37];
+  //uuid_unparse(u.uuid, b);
+  return out << u.uuid;
+}
+
+inline bool operator==(const uuid_d& l, const uuid_d& r) {
+  return l.uuid == r.uuid;
+}
+
+inline bool operator!=(const uuid_d& l, const uuid_d& r) {
+  return l.uuid != r.uuid;
+}
+#else
 extern "C" {
 #include <uuid/uuid.h>
 #include <unistd.h>
@@ -56,6 +120,6 @@ inline bool operator==(const uuid_d& l, const uuid_d& r) {
 inline bool operator!=(const uuid_d& l, const uuid_d& r) {
   return uuid_compare(l.uuid, r.uuid) != 0;
 }
-
+#endif

 #endif
diff --git a/src/messages/MStatfsReply.h b/src/messages/MStatfsReply.h
index 8ceec9c..40a5bdd 100644
--- a/src/messages/MStatfsReply.h
+++ b/src/messages/MStatfsReply.h
@@ -22,7 +22,7 @@ public:

   MStatfsReply() : Message(CEPH_MSG_STATFS_REPLY) {}
   MStatfsReply(uuid_d &f, tid_t t, epoch_t epoch) : Message(CEPH_MSG_STATFS_REPLY) {
-    memcpy(&h.fsid, f.uuid, sizeof(h.fsid));
+    memcpy(&h.fsid, &f.uuid, sizeof(h.fsid));
     header.tid = t;
     h.version = epoch;
   }


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-13 22:33             ` James Harper
@ 2013-11-26  3:46               ` Noah Watkins
  2013-11-26  5:12                 ` Sage Weil
  2013-11-26  5:52                 ` James Harper
  0 siblings, 2 replies; 14+ messages in thread
From: Noah Watkins @ 2013-11-26  3:46 UTC (permalink / raw)
  To: James Harper; +Cc: Sage Weil, ceph-devel

James,

I’m using uuid.begin()/end() to grab the 16-byte representation of the UUID. Did you figure out how to populate a boost::uuid_t from the bytes? In particular, I’m referring to FileJournal::decode. 

Actually, I suppose that any Ceph usage of the 16-byte representation should be replaced using the Boost serialization of uuid_t?

Thanks,
-Noah

On Nov 13, 2013, at 2:33 PM, James Harper <james.harper@bendigoit.com.au> wrote:

> Patch follows. When I wrote it I was just thinking it would be used for win32 build, hence the #ifdef. As I said before, it compiles but I haven't tested it. I can clean it up a bit and resend it with a signed-off-by if anyone wants to pick it up and follow it through sooner than I can. I don't know how boost behaves if the uuid parse fails (exception maybe?) so that would need resolving too.
> 
> In addition, a bunch of ceph files include the libuuid header directly, even though all the ones I've found don't appear to need it, so they need to be fixed for a clean compile under win32, and to remove dependency on libuuid. There may also be other cases that need work, in particular anything that memcpy's into the 16 byte uuid directly. See patch for MStatfsReply.h where a minor tweak was necessary.
> 
> (if anyone is interested, I have librados and librbd compiling under mingw32, but I can't get boost to build its thread library so I don't get a clean link, and there are probably other link errors too. I've run out of time for doing much more on this for the moment)
> 
> James
> 
> diff --git a/src/include/uuid.h b/src/include/uuid.h
> index 942b807..201ac76 100644
> --- a/src/include/uuid.h
> +++ b/src/include/uuid.h
> @@ -8,6 +8,70 @@
> #include "encoding.h"
> #include <ostream>
> 
> +#if defined(_WIN32)
> +
> +#include <boost/lexical_cast.hpp>
> +#include <boost/uuid/uuid.hpp>
> +#include <boost/uuid/uuid_generators.hpp>
> +#include <boost/uuid/uuid_io.hpp>
> +#include <string>
> +
> +struct uuid_d {
> +  boost::uuids::uuid uuid;
> +
> +  uuid_d() {
> +    uuid = boost::uuids::nil_uuid();
> +  }
> +
> +  bool is_zero() const {
> +    return uuid.is_nil();
> +    //return boost::uuids::uuid::is_nil(uuid);
> +  }
> +
> +  void generate_random() {
> +    boost::uuids::random_generator gen;
> +    uuid = gen();
> +  }
> +
> +  bool parse(const char *s) {
> +    boost::uuids::string_generator gen;
> +    uuid = gen(s);
> +    return true;
> +    // what happens if parse fails?
> +  }
> +  void print(char *s) {
> +    std::string str = boost::lexical_cast<std::string>(uuid);
> +    memcpy(s, str.c_str(), 37);
> +  }
> +
> +  void encode(bufferlist& bl) const {
> +    ::encode_raw(uuid, bl);
> +  }
> +  void decode(bufferlist::iterator& p) const {
> +    ::decode_raw(uuid, p);
> +  }
> +
> +  uuid_d& operator=(const uuid_d& r) {
> +    uuid = r.uuid;
> +    return *this;
> +  }
> +};
> +WRITE_CLASS_ENCODER(uuid_d)
> +
> +inline std::ostream& operator<<(std::ostream& out, const uuid_d& u) {
> +  //char b[37];
> diff --git a/src/include/uuid.h b/src/include/uuid.h
> index 942b807..201ac76 100644
> --- a/src/include/uuid.h
> +++ b/src/include/uuid.h
> @@ -8,6 +8,70 @@
> #include "encoding.h"
> #include <ostream>
> 
> +#if defined(_WIN32)
> +
> +#include <boost/lexical_cast.hpp>
> +#include <boost/uuid/uuid.hpp>
> +#include <boost/uuid/uuid_generators.hpp>
> +#include <boost/uuid/uuid_io.hpp>
> +#include <string>
> +
> +struct uuid_d {
> +  boost::uuids::uuid uuid;
> +
> +  uuid_d() {
> +    uuid = boost::uuids::nil_uuid();
> +  }
> +
> +  bool is_zero() const {
> +    return uuid.is_nil();
> +    //return boost::uuids::uuid::is_nil(uuid);
> +  }
> +
> +  void generate_random() {
> +    boost::uuids::random_generator gen;
> +    uuid = gen();
> +  }
> +
> +  bool parse(const char *s) {
> +    boost::uuids::string_generator gen;
> +    uuid = gen(s);
> +    return true;
> +    // what happens if parse fails?
> +  }
> +  void print(char *s) {
> +    std::string str = boost::lexical_cast<std::string>(uuid);
> +    memcpy(s, str.c_str(), 37);
> +  }
> +
> +  void encode(bufferlist& bl) const {
> +    ::encode_raw(uuid, bl);
> +  }
> +  void decode(bufferlist::iterator& p) const {
> +    ::decode_raw(uuid, p);
> +  }
> +
> +  uuid_d& operator=(const uuid_d& r) {
> +    uuid = r.uuid;
> +    return *this;
> +  }
> +};
> +WRITE_CLASS_ENCODER(uuid_d)
> +
> +inline std::ostream& operator<<(std::ostream& out, const uuid_d& u) {
> +  //char b[37];
> +  //uuid_unparse(u.uuid, b);
> +  return out << u.uuid;
> +}
> +
> +inline bool operator==(const uuid_d& l, const uuid_d& r) {
> +  return l.uuid == r.uuid;
> +}
> +
> +inline bool operator!=(const uuid_d& l, const uuid_d& r) {
> +  return l.uuid != r.uuid;
> +}
> +#else
> extern "C" {
> #include <uuid/uuid.h>
> #include <unistd.h>
> @@ -56,6 +120,6 @@ inline bool operator==(const uuid_d& l, const uuid_d& r) {
> inline bool operator!=(const uuid_d& l, const uuid_d& r) {
>   return uuid_compare(l.uuid, r.uuid) != 0;
> }
> -
> +#endif
> 
> #endif
> diff --git a/src/messages/MStatfsReply.h b/src/messages/MStatfsReply.h
> index 8ceec9c..40a5bdd 100644
> --- a/src/messages/MStatfsReply.h
> +++ b/src/messages/MStatfsReply.h
> @@ -22,7 +22,7 @@ public:
> 
>   MStatfsReply() : Message(CEPH_MSG_STATFS_REPLY) {}
>   MStatfsReply(uuid_d &f, tid_t t, epoch_t epoch) : Message(CEPH_MSG_STATFS_REPLY) {
> -    memcpy(&h.fsid, f.uuid, sizeof(h.fsid));
> +    memcpy(&h.fsid, &f.uuid, sizeof(h.fsid));
>     header.tid = t;
>     h.version = epoch;
>   }
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-26  3:46               ` Noah Watkins
@ 2013-11-26  5:12                 ` Sage Weil
  2013-11-26  5:52                 ` James Harper
  1 sibling, 0 replies; 14+ messages in thread
From: Sage Weil @ 2013-11-26  5:12 UTC (permalink / raw)
  To: Noah Watkins; +Cc: James Harper, ceph-devel

On Mon, 25 Nov 2013, Noah Watkins wrote:
> James,
> 
> I?m using uuid.begin()/end() to grab the 16-byte representation of the UUID. Did you figure out how to populate a boost::uuid_t from the bytes? In particular, I?m referring to FileJournal::decode. 
> 
> Actually, I suppose that any Ceph usage of the 16-byte representation should be replaced using the Boost serialization of uuid_t?

I suspect it is a trivial mapping, but you should render it as a string 
using both the old class and the boost code to confirm it is equivalent.

(For my part, I had no idea the uuid was anything other than a bunch of 
random bits until I looked at the boost code.  Now the weird grouping of 
digits/dashes makes some sense.)

sage


> 
> Thanks,
> -Noah
> 
> On Nov 13, 2013, at 2:33 PM, James Harper <james.harper@bendigoit.com.au> wrote:
> 
> > Patch follows. When I wrote it I was just thinking it would be used for win32 build, hence the #ifdef. As I said before, it compiles but I haven't tested it. I can clean it up a bit and resend it with a signed-off-by if anyone wants to pick it up and follow it through sooner than I can. I don't know how boost behaves if the uuid parse fails (exception maybe?) so that would need resolving too.
> > 
> > In addition, a bunch of ceph files include the libuuid header directly, even though all the ones I've found don't appear to need it, so they need to be fixed for a clean compile under win32, and to remove dependency on libuuid. There may also be other cases that need work, in particular anything that memcpy's into the 16 byte uuid directly. See patch for MStatfsReply.h where a minor tweak was necessary.
> > 
> > (if anyone is interested, I have librados and librbd compiling under mingw32, but I can't get boost to build its thread library so I don't get a clean link, and there are probably other link errors too. I've run out of time for doing much more on this for the moment)
> > 
> > James
> > 
> > diff --git a/src/include/uuid.h b/src/include/uuid.h
> > index 942b807..201ac76 100644
> > --- a/src/include/uuid.h
> > +++ b/src/include/uuid.h
> > @@ -8,6 +8,70 @@
> > #include "encoding.h"
> > #include <ostream>
> > 
> > +#if defined(_WIN32)
> > +
> > +#include <boost/lexical_cast.hpp>
> > +#include <boost/uuid/uuid.hpp>
> > +#include <boost/uuid/uuid_generators.hpp>
> > +#include <boost/uuid/uuid_io.hpp>
> > +#include <string>
> > +
> > +struct uuid_d {
> > +  boost::uuids::uuid uuid;
> > +
> > +  uuid_d() {
> > +    uuid = boost::uuids::nil_uuid();
> > +  }
> > +
> > +  bool is_zero() const {
> > +    return uuid.is_nil();
> > +    //return boost::uuids::uuid::is_nil(uuid);
> > +  }
> > +
> > +  void generate_random() {
> > +    boost::uuids::random_generator gen;
> > +    uuid = gen();
> > +  }
> > +
> > +  bool parse(const char *s) {
> > +    boost::uuids::string_generator gen;
> > +    uuid = gen(s);
> > +    return true;
> > +    // what happens if parse fails?
> > +  }
> > +  void print(char *s) {
> > +    std::string str = boost::lexical_cast<std::string>(uuid);
> > +    memcpy(s, str.c_str(), 37);
> > +  }
> > +
> > +  void encode(bufferlist& bl) const {
> > +    ::encode_raw(uuid, bl);
> > +  }
> > +  void decode(bufferlist::iterator& p) const {
> > +    ::decode_raw(uuid, p);
> > +  }
> > +
> > +  uuid_d& operator=(const uuid_d& r) {
> > +    uuid = r.uuid;
> > +    return *this;
> > +  }
> > +};
> > +WRITE_CLASS_ENCODER(uuid_d)
> > +
> > +inline std::ostream& operator<<(std::ostream& out, const uuid_d& u) {
> > +  //char b[37];
> > diff --git a/src/include/uuid.h b/src/include/uuid.h
> > index 942b807..201ac76 100644
> > --- a/src/include/uuid.h
> > +++ b/src/include/uuid.h
> > @@ -8,6 +8,70 @@
> > #include "encoding.h"
> > #include <ostream>
> > 
> > +#if defined(_WIN32)
> > +
> > +#include <boost/lexical_cast.hpp>
> > +#include <boost/uuid/uuid.hpp>
> > +#include <boost/uuid/uuid_generators.hpp>
> > +#include <boost/uuid/uuid_io.hpp>
> > +#include <string>
> > +
> > +struct uuid_d {
> > +  boost::uuids::uuid uuid;
> > +
> > +  uuid_d() {
> > +    uuid = boost::uuids::nil_uuid();
> > +  }
> > +
> > +  bool is_zero() const {
> > +    return uuid.is_nil();
> > +    //return boost::uuids::uuid::is_nil(uuid);
> > +  }
> > +
> > +  void generate_random() {
> > +    boost::uuids::random_generator gen;
> > +    uuid = gen();
> > +  }
> > +
> > +  bool parse(const char *s) {
> > +    boost::uuids::string_generator gen;
> > +    uuid = gen(s);
> > +    return true;
> > +    // what happens if parse fails?
> > +  }
> > +  void print(char *s) {
> > +    std::string str = boost::lexical_cast<std::string>(uuid);
> > +    memcpy(s, str.c_str(), 37);
> > +  }
> > +
> > +  void encode(bufferlist& bl) const {
> > +    ::encode_raw(uuid, bl);
> > +  }
> > +  void decode(bufferlist::iterator& p) const {
> > +    ::decode_raw(uuid, p);
> > +  }
> > +
> > +  uuid_d& operator=(const uuid_d& r) {
> > +    uuid = r.uuid;
> > +    return *this;
> > +  }
> > +};
> > +WRITE_CLASS_ENCODER(uuid_d)
> > +
> > +inline std::ostream& operator<<(std::ostream& out, const uuid_d& u) {
> > +  //char b[37];
> > +  //uuid_unparse(u.uuid, b);
> > +  return out << u.uuid;
> > +}
> > +
> > +inline bool operator==(const uuid_d& l, const uuid_d& r) {
> > +  return l.uuid == r.uuid;
> > +}
> > +
> > +inline bool operator!=(const uuid_d& l, const uuid_d& r) {
> > +  return l.uuid != r.uuid;
> > +}
> > +#else
> > extern "C" {
> > #include <uuid/uuid.h>
> > #include <unistd.h>
> > @@ -56,6 +120,6 @@ inline bool operator==(const uuid_d& l, const uuid_d& r) {
> > inline bool operator!=(const uuid_d& l, const uuid_d& r) {
> >   return uuid_compare(l.uuid, r.uuid) != 0;
> > }
> > -
> > +#endif
> > 
> > #endif
> > diff --git a/src/messages/MStatfsReply.h b/src/messages/MStatfsReply.h
> > index 8ceec9c..40a5bdd 100644
> > --- a/src/messages/MStatfsReply.h
> > +++ b/src/messages/MStatfsReply.h
> > @@ -22,7 +22,7 @@ public:
> > 
> >   MStatfsReply() : Message(CEPH_MSG_STATFS_REPLY) {}
> >   MStatfsReply(uuid_d &f, tid_t t, epoch_t epoch) : Message(CEPH_MSG_STATFS_REPLY) {
> > -    memcpy(&h.fsid, f.uuid, sizeof(h.fsid));
> > +    memcpy(&h.fsid, &f.uuid, sizeof(h.fsid));
> >     header.tid = t;
> >     h.version = epoch;
> >   }
> > 
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: libuuid vs boost uuid
  2013-11-26  3:46               ` Noah Watkins
  2013-11-26  5:12                 ` Sage Weil
@ 2013-11-26  5:52                 ` James Harper
  2013-11-26 16:44                   ` Noah Watkins
  1 sibling, 1 reply; 14+ messages in thread
From: James Harper @ 2013-11-26  5:52 UTC (permalink / raw)
  To: Noah Watkins; +Cc: Sage Weil, ceph-devel

> 
> James,
> 
> I'm using uuid.begin()/end() to grab the 16-byte representation of the UUID.
> Did you figure out how to populate a boost::uuid_t from the bytes? In
> particular, I'm referring to FileJournal::decode.
> 
> Actually, I suppose that any Ceph usage of the 16-byte representation should
> be replaced using the Boost serialization of uuid_t?
> 

As I said I haven't actually tested it, apart from that I have librbd working under Windows now ("rbd ls" and "rbd export" both work but I don't know if they actually do anything with uuid's...)

My patch to MStatfsReply.h to make it compile is:

diff --git a/src/messages/MStatfsReply.h b/src/messages/MStatfsReply.h
index 8ceec9c..40a5bdd 100644
--- a/src/messages/MStatfsReply.h
+++ b/src/messages/MStatfsReply.h
@@ -22,7 +22,7 @@ public:

   MStatfsReply() : Message(CEPH_MSG_STATFS_REPLY) {}
   MStatfsReply(uuid_d &f, tid_t t, epoch_t epoch) : Message(CEPH_MSG_STATFS_REPLY) {
-    memcpy(&h.fsid, f.uuid, sizeof(h.fsid));
+    memcpy(&h.fsid, &f.uuid, sizeof(h.fsid));
     header.tid = t;
     h.version = epoch;
   }

So assuming this actually works, the uuid bytes are accessible as per the above.

James


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: libuuid vs boost uuid
  2013-11-26  5:52                 ` James Harper
@ 2013-11-26 16:44                   ` Noah Watkins
  0 siblings, 0 replies; 14+ messages in thread
From: Noah Watkins @ 2013-11-26 16:44 UTC (permalink / raw)
  To: James Harper; +Cc: Sage Weil, ceph-devel

I put up a patch here for review

https://github.com/ceph/ceph/pull/875/files

It seems ok as long as boost doesn’t ever try to change their internal representation, which in this patch we reach in an grab for the 16 octet representation. Why not just grab a copy of libuuid from util-linux and keep it in tree?

On Nov 25, 2013, at 9:52 PM, James Harper <james.harper@bendigoit.com.au> wrote:

>> 
>> James,
>> 
>> I'm using uuid.begin()/end() to grab the 16-byte representation of the UUID.
>> Did you figure out how to populate a boost::uuid_t from the bytes? In
>> particular, I'm referring to FileJournal::decode.
>> 
>> Actually, I suppose that any Ceph usage of the 16-byte representation should
>> be replaced using the Boost serialization of uuid_t?
>> 
> 
> As I said I haven't actually tested it, apart from that I have librbd working under Windows now ("rbd ls" and "rbd export" both work but I don't know if they actually do anything with uuid's...)
> 
> My patch to MStatfsReply.h to make it compile is:
> 
> diff --git a/src/messages/MStatfsReply.h b/src/messages/MStatfsReply.h
> index 8ceec9c..40a5bdd 100644
> --- a/src/messages/MStatfsReply.h
> +++ b/src/messages/MStatfsReply.h
> @@ -22,7 +22,7 @@ public:
> 
>   MStatfsReply() : Message(CEPH_MSG_STATFS_REPLY) {}
>   MStatfsReply(uuid_d &f, tid_t t, epoch_t epoch) : Message(CEPH_MSG_STATFS_REPLY) {
> -    memcpy(&h.fsid, f.uuid, sizeof(h.fsid));
> +    memcpy(&h.fsid, &f.uuid, sizeof(h.fsid));
>     header.tid = t;
>     h.version = epoch;
>   }
> 
> So assuming this actually works, the uuid bytes are accessible as per the above.
> 
> James
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-11-26 16:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-09  5:59 libuuid vs boost uuid James Harper
2013-11-09  6:43 ` Sage Weil
2013-11-09 16:41   ` Noah Watkins
2013-11-09 17:25     ` asomers
2013-11-10  1:44   ` James Harper
2013-11-10  5:22     ` Sage Weil
2013-11-13 15:07       ` Noah Watkins
2013-11-13 21:42         ` James Harper
2013-11-13 22:03           ` Noah Watkins
2013-11-13 22:33             ` James Harper
2013-11-26  3:46               ` Noah Watkins
2013-11-26  5:12                 ` Sage Weil
2013-11-26  5:52                 ` James Harper
2013-11-26 16:44                   ` Noah Watkins

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.