CEPH filesystem development
 help / color / mirror / Atom feed
From: Mykola Golub <mgolub@mirantis.com>
To: Josh Durgin <jdurgin@redhat.com>
Cc: Sage Weil <sweil@redhat.com>,
	Jason Dillaman <dillaman@redhat.com>,
	ceph-devel@vger.kernel.org
Subject: Re: CEPH_RBD_API: options on image create
Date: Fri, 16 Oct 2015 08:32:12 +0300	[thread overview]
Message-ID: <20151016053211.GA21439@gmail.com> (raw)
In-Reply-To: <561FF0A4.3000805@redhat.com>

Thank you all for your comments! I will come back with PR and pull
request.

-- 
Mykola Golub

On Thu, Oct 15, 2015 at 11:29:56AM -0700, Josh Durgin wrote:
> On 10/15/2015 06:45 AM, Sage Weil wrote:
> >On Thu, 15 Oct 2015, Mykola Golub wrote:
> >>On Thu, Oct 15, 2015 at 08:47:58AM -0400, Jason Dillaman wrote:
> >>>>
> >>>>But we don't need them to match between different platforms, no? Is
> >>>>linking 64bit code with 32bit possible (supported)?
> >>>>
> >>>>Also, for this particular (char*) case, length would actually be the
> >>>>length of the string, not the pointer length. From my example:
> >>>>
> >>>>const char* journal_object_pool = "journal";
> >>>>r = rbd_image_options_set(opts, RBD_OPTION_JOURNAL_OBJECT_POOL,
> >>>>                           journal_object_pool, strlen(journal_object_pool) +
> >>>>                           1);
> >>>>
> >>>
> >>>My original example was a string of length 4 vs a 4-byte int, but
> >>>you said you were thinking of sizeof(type) instead.  I think this
> >>>style of interface is great if you need to pass any arbitrary data
> >>>along, but will we ever expect to pass along anything besides a
> >>>string or an (u)int(32/64)?
> >>
> >>I don't know, sure you have much more experience here, so if you
> >>hardly expect other types in future, I would be rather then for
> >>rbd_image_options_set_{uint64,str}() functions.
> 
> This is my favorite option too.
> 
> >Having a str and int variant seems like the best path to me.  Maybe int64
> >though, so that signed values are possible?
> 
> Don't think we need any signed values currently. It doesn't cause any
> ABI problems to add signed ints or other types later, since it'll
> already be overloaded in C++, and for C they need to be new functions
> anyway.
> 
> >>>On the flip-side, what will the C++ interface look like?  An
> >>>equivalent API would imply passing a boost::any.  While certainly
> >>>future-proof, something about that doesn't sit right with me as an
> >>>API.  I think I would lean more towards something like xyz_set(const
> >>>std::string&), xyz_set(uint64_t), et al.
> >>
> >>For C++ I was also thinking about xyz_set(const std::string&),
> >>xyz_set(uint64_t) variants, i.e:
> >>
> >>int rbd::Image::options::set(int optname, const std::string& val);
> >>int rbd::Image::options::set(int optname, uint64_t val);
> 
> Sounds good to me.
> 
> Josh
> 
> --
> 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

  reply	other threads:[~2015-10-16  5:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-30  6:50 CEPH_RBD_API: options on image create Mykola Golub
2015-10-14 19:34 ` Jason Dillaman
2015-10-15  3:12   ` Josh Durgin
2015-10-15  6:56     ` Mykola Golub
2015-10-15  6:33   ` Mykola Golub
2015-10-15 12:05     ` Jason Dillaman
2015-10-15 12:33       ` Mykola Golub
2015-10-15 12:47         ` Jason Dillaman
2015-10-15 13:28           ` Mykola Golub
2015-10-15 13:45             ` Sage Weil
2015-10-15 18:29               ` Josh Durgin
2015-10-16  5:32                 ` Mykola Golub [this message]
2015-10-24 13:16                   ` Mykola Golub

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151016053211.GA21439@gmail.com \
    --to=mgolub@mirantis.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=dillaman@redhat.com \
    --cc=jdurgin@redhat.com \
    --cc=sweil@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox