qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] Tree broken by nbd: support feature negotiation commit.
@ 2011-09-21  7:27 Brad
  2011-09-21  7:32 ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Brad @ 2011-09-21  7:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini

The following commit..

nbd: support feature negotiation

nbd supports writing flags in bytes 24...27 of the header,
and uses that for the read-only flag.  Add support for it
in qemu-nbd.

breaks the tree on what looks like anything but Linux.

Besides the obvious issue..

nbd.c:443: error: conflicting types for 'nbd_init'
nbd.h:71: error: previous declaration of 'nbd_init' was here

The changing of #ifndef _WIN32 to #ifdef __linux__ in nbd.c also
looks questionable to me.

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

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

* Re: [Qemu-devel] Tree broken by nbd: support feature negotiation commit.
  2011-09-21  7:27 [Qemu-devel] Tree broken by nbd: support feature negotiation commit Brad
@ 2011-09-21  7:32 ` Paolo Bonzini
  2011-09-21  7:42   ` Kevin Wolf
  2011-09-21  7:43   ` Brad
  0 siblings, 2 replies; 6+ messages in thread
From: Paolo Bonzini @ 2011-09-21  7:32 UTC (permalink / raw)
  To: Brad; +Cc: Kevin Wolf, qemu-devel

On 09/21/2011 09:27 AM, Brad wrote:
> Besides the obvious issue..
>
> nbd.c:443: error: conflicting types for 'nbd_init'
> nbd.h:71: error: previous declaration of 'nbd_init' was here

Oops, thanks for pointing it out to me.

> The changing of #ifndef _WIN32 to #ifdef __linux__ in nbd.c also
> looks questionable to me.

It is not portable code, and (unlike the rest of qemu-nbd and the 
block/nbd.c protocol) not meant to be portable.  Are BLKROSET (defined 
in linux/fs.h) and the whole set of NBD ioctls available under OpenBSD?

Paolo

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

* Re: [Qemu-devel] Tree broken by nbd: support feature negotiation commit.
  2011-09-21  7:32 ` Paolo Bonzini
@ 2011-09-21  7:42   ` Kevin Wolf
  2011-09-21  7:43   ` Brad
  1 sibling, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2011-09-21  7:42 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Brad

Am 21.09.2011 09:32, schrieb Paolo Bonzini:
> On 09/21/2011 09:27 AM, Brad wrote:
>> Besides the obvious issue..
>>
>> nbd.c:443: error: conflicting types for 'nbd_init'
>> nbd.h:71: error: previous declaration of 'nbd_init' was here
> 
> Oops, thanks for pointing it out to me.

Can you please send a fix? I'll do another pull request today then.

And maybe I should really run a mingw build each time. Or get a BSD VM.

Kevin

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

* Re: [Qemu-devel] Tree broken by nbd: support feature negotiation commit.
  2011-09-21  7:32 ` Paolo Bonzini
  2011-09-21  7:42   ` Kevin Wolf
@ 2011-09-21  7:43   ` Brad
  2011-09-21  9:21     ` Paolo Bonzini
  1 sibling, 1 reply; 6+ messages in thread
From: Brad @ 2011-09-21  7:43 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Kevin Wolf, qemu-devel

On 21/09/11 3:32 AM, Paolo Bonzini wrote:
> On 09/21/2011 09:27 AM, Brad wrote:
>> Besides the obvious issue..
>>
>> nbd.c:443: error: conflicting types for 'nbd_init'
>> nbd.h:71: error: previous declaration of 'nbd_init' was here
>
> Oops, thanks for pointing it out to me.
>
>> The changing of #ifndef _WIN32 to #ifdef __linux__ in nbd.c also
>> looks questionable to me.
>
> It is not portable code, and (unlike the rest of qemu-nbd and the
> block/nbd.c protocol) not meant to be portable. Are BLKROSET (defined in
> linux/fs.h) and the whole set of NBD ioctls available under OpenBSD?

Ok. What confused me a bit is that particular code path before your 
commit was being built on anything but Windows but is now Linux only.
No we don't have BLKROSET. So am I to understand that even before this
particular commit that this code was only supported on Linux? I honestly
have no familiarity with NBD.


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

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

* Re: [Qemu-devel] Tree broken by nbd: support feature negotiation commit.
  2011-09-21  7:43   ` Brad
@ 2011-09-21  9:21     ` Paolo Bonzini
  2011-09-21  9:23       ` Brad
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2011-09-21  9:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, blauwirbel, brad

On 09/21/2011 09:43 AM, Brad wrote:
>> It is not portable code, and (unlike the rest of qemu-nbd and the
>> block/nbd.c protocol) not meant to be portable. Are BLKROSET (defined in
>> linux/fs.h) and the whole set of NBD ioctls available under OpenBSD?
> 
> Ok. What confused me a bit is that particular code path before your 
> commit was being built on anything but Windows but is now Linux only.
> No we don't have BLKROSET. So am I to understand that even before this
> particular commit that this code was only supported on Linux?

Yes.

Here's a fix.

Paolo

----------------------- 8< -----------------------

From: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed, 21 Sep 2011 09:34:12 +0200
Subject: [PATCH] nbd: fix non-Linux build failure

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c |   29 ++++++++++++-----------------
 1 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/nbd.c b/nbd.c
index 595f4d8..9810f99 100644
--- a/nbd.c
+++ b/nbd.c
@@ -437,7 +447,7 @@ int nbd_client(int fd)
     return ret;
 }
 #else
-int nbd_init(int fd, int csock, off_t size, size_t blocksize)
+int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize)
 {
     errno = ENOTSUP;
     return -1;
-- 
1.7.6

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

* Re: [Qemu-devel] Tree broken by nbd: support feature negotiation commit.
  2011-09-21  9:21     ` Paolo Bonzini
@ 2011-09-21  9:23       ` Brad
  0 siblings, 0 replies; 6+ messages in thread
From: Brad @ 2011-09-21  9:23 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kwolf, blauwirbel, qemu-devel

On 21/09/11 5:21 AM, Paolo Bonzini wrote:
> On 09/21/2011 09:43 AM, Brad wrote:
>>> It is not portable code, and (unlike the rest of qemu-nbd and the
>>> block/nbd.c protocol) not meant to be portable. Are BLKROSET (defined in
>>> linux/fs.h) and the whole set of NBD ioctls available under OpenBSD?
>>
>> Ok. What confused me a bit is that particular code path before your
>> commit was being built on anything but Windows but is now Linux only.
>> No we don't have BLKROSET. So am I to understand that even before this
>> particular commit that this code was only supported on Linux?
>
> Yes.
>
> Here's a fix.

Yes, this is what I also came up with to get the tree to build for me.

> Paolo
>
> ----------------------- 8<  -----------------------
>
> From: Paolo Bonzini<pbonzini@redhat.com>
> Date: Wed, 21 Sep 2011 09:34:12 +0200
> Subject: [PATCH] nbd: fix non-Linux build failure
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   nbd.c |   29 ++++++++++++-----------------
>   1 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/nbd.c b/nbd.c
> index 595f4d8..9810f99 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -437,7 +447,7 @@ int nbd_client(int fd)
>       return ret;
>   }
>   #else
> -int nbd_init(int fd, int csock, off_t size, size_t blocksize)
> +int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize)
>   {
>       errno = ENOTSUP;
>       return -1;


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.

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

end of thread, other threads:[~2011-09-21  9:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-21  7:27 [Qemu-devel] Tree broken by nbd: support feature negotiation commit Brad
2011-09-21  7:32 ` Paolo Bonzini
2011-09-21  7:42   ` Kevin Wolf
2011-09-21  7:43   ` Brad
2011-09-21  9:21     ` Paolo Bonzini
2011-09-21  9:23       ` Brad

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).