From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58453)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WHztS-00088i-GH
	for qemu-devel@nongnu.org; Mon, 24 Feb 2014 13:04:48 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WHztM-0002Hg-DS
	for qemu-devel@nongnu.org; Mon, 24 Feb 2014 13:04:42 -0500
Received: from mx4-phx2.redhat.com ([209.132.183.25]:50901)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WHztM-0002Hc-1s
	for qemu-devel@nongnu.org; Mon, 24 Feb 2014 13:04:36 -0500
Date: Mon, 24 Feb 2014 13:04:34 -0500 (EST)
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <1290712841.8456749.1393265074209.JavaMail.zimbra@redhat.com>
In-Reply-To: <CAKx3GuwW0a-To4E0ptbyPstAspn8XhF0-6NmXxCBsqgi71ZqEQ@mail.gmail.com>
References: <1393233201-3636-1-git-send-email-orx.goshen@intel.com>
	<530B0ECB.8080608@redhat.com>
	<CAKx3GuwW0a-To4E0ptbyPstAspn8XhF0-6NmXxCBsqgi71ZqEQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] Support NBD client under win32/MinGW
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Or Goshen <oberonc@gmail.com>
Cc: qemu-devel@nongnu.org


> I never used/tested sheepdog under windows with my patch, so I wouldnt know
> if it doesnt have any additional bugs under windows.

If it compiles, you can include it.  Worst of all, people will find bugs.
It used to be enabled for Windows when NBD was also enabled (before the
introduction of asynchronous I/O).

This applies not just to sheepdog, also to curl and libiscsi.

Paolo

> BTW, this patch is on behalf of Intel Corp
> 
> 
> On Mon, Feb 24, 2014 at 11:20 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > Il 24/02/2014 10:13, Or Goshen ha scritto:
> >
> >
> >> +               /* Create event */
> >> +               HANDLE event = WSACreateEvent();
> >> +               long lNetworkEvents = 0;
> >> +
> >> +               if (node->io_read)
> >> +                       lNetworkEvents |= FD_READ;
> >> +               if (node->io_write)
> >> +                       lNetworkEvents |= FD_WRITE;
> >> +
> >> +               WSAEventSelect(node->pfd.fd, event, lNetworkEvents);
> >> +               node->e = (EventNotifier *)event;
> >>
> >
> > As mentioned on IRC, I'd like you to try using the EventNotifier of the
> > AioContext.  You shouldn't need any of these casts.
> >
> >
> >   ifeq ($(CONFIG_POSIX),y)
> >> -block-obj-y += nbd.o nbd-client.o sheepdog.o
> >> +block-obj-y += nbd-client.o sheepdog.o
> >>
> >
> > nbd-client.c must be compiled on Windows, too.
> >
> > In fact I think you can just drop the ifeq/endif directives.
> >
> >
> >   block-obj-$(CONFIG_LIBISCSI) += iscsi.o
> >>  block-obj-$(CONFIG_CURL) += curl.o
> >>  block-obj-$(CONFIG_RBD) += rbd.o
> >>
> >
> > Also, the patch has wrong indentation.  We use 4 space indentation, and
> > never use tabs.
> >
> > Paolo
> >
> 
> 
> 
> --
>    Or Goshen
>    CTO
>    Cerbercomm
>    Mob: 972-50-9182418
>