Xen devel - Oct 2011 - [PATCH] QEMU: remove blktap when building for NetBSD

From: Charlie Root <root@loki.(none)>

NetBSD has no blktap support, so remove the use of the blktap if the OS is
NetBSD.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
---
 hw/xen_machine_pv.c |    2 +-
 xen-config-host.h   |    2 +-
 xen-hooks.mak       |    2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index 0004904..c09adbb 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -47,7 +47,7 @@ static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
     CPUState *env;
     uint32_t domid_target;
 
-#ifndef CONFIG_STUBDOM
+#if !defined(CONFIG_STUBDOM) && !defined(__NetBSD__)
     /* Initialize tapdisk client */
     init_blktap();
 #endif
diff --git a/xen-config-host.h b/xen-config-host.h
index f50c3aa..818f25d 100644
--- a/xen-config-host.h
+++ b/xen-config-host.h
@@ -18,7 +18,7 @@ extern int domid, domid_backend;
 
 #include "xenctrl.h"
 #include "xs.h"
-#ifndef CONFIG_STUBDOM
+#if !defined(CONFIG_STUBDOM) && !defined(__NetBSD__)
 #include "blktaplib.h"
 #endif
 
diff --git a/xen-hooks.mak b/xen-hooks.mak
index 253915d..b55f45b 100644
--- a/xen-hooks.mak
+++ b/xen-hooks.mak
@@ -46,11 +46,13 @@ CONFIG_SDL CONFIG_AUDIO OBJS += xenfbfront.o
 else
+ifndef CONFIG_NetBSD
 CPPFLAGS+= -I$(XEN_ROOT)/tools/blktap/lib
 LIBS += -L$(XEN_ROOT)/tools/blktap/lib -lblktap
 OBJS += xen_blktap.o
 OBJS += tpm_tis.o
 endif
+endif
 
 ifdef CONFIG_STUBDOM
 CONFIG_PASSTHROUGH=1
-- 
1.7.6.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2011-10-04 at 15:13 +0100, Roger Pau Monne
wrote:
> From: Charlie Root <root@loki.(none)>
Really?
> 
> NetBSD has no blktap support, so remove the use of the blktap if the OS is
NetBSD.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> ---
>  hw/xen_machine_pv.c |    2 +-
>  xen-config-host.h   |    2 +-
>  xen-hooks.mak       |    2 ++
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
> index 0004904..c09adbb 100644
> --- a/hw/xen_machine_pv.c
> +++ b/hw/xen_machine_pv.c
> @@ -47,7 +47,7 @@ static void xen_init_pv(ram_addr_t ram_size, int
vga_ram_size,
>      CPUState *env;
>      uint32_t domid_target;
>  
> -#ifndef CONFIG_STUBDOM
> +#if !defined(CONFIG_STUBDOM) && !defined(__NetBSD__)
>      /* Initialize tapdisk client */
>      init_blktap();
>  #endif
> diff --git a/xen-config-host.h b/xen-config-host.h
> index f50c3aa..818f25d 100644
> --- a/xen-config-host.h
> +++ b/xen-config-host.h
> @@ -18,7 +18,7 @@ extern int domid, domid_backend;
>  
>  #include "xenctrl.h"
>  #include "xs.h"
> -#ifndef CONFIG_STUBDOM
> +#if !defined(CONFIG_STUBDOM) && !defined(__NetBSD__)
>  #include "blktaplib.h"
>  #endif
>  
> diff --git a/xen-hooks.mak b/xen-hooks.mak
> index 253915d..b55f45b 100644
> --- a/xen-hooks.mak
> +++ b/xen-hooks.mak
> @@ -46,11 +46,13 @@ CONFIG_SDL>  CONFIG_AUDIO>  OBJS += xenfbfront.o
>  else
> +ifndef CONFIG_NetBSD
>  CPPFLAGS+= -I$(XEN_ROOT)/tools/blktap/lib
>  LIBS += -L$(XEN_ROOT)/tools/blktap/lib -lblktap
>  OBJS += xen_blktap.o
>  OBJS += tpm_tis.o
>  endif
> +endif
>  
>  ifdef CONFIG_STUBDOM
>  CONFIG_PASSTHROUGH=1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> Really?
NetBSD added this files to blktap lib to be able to compile it, since
Qemu needed it:

http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/pkgsrc/sysutils/xentools41/files/blk_netbsd.c?rev=1.1.1.1&content-type=text/plain&only_with_tag=MAIN
http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/pkgsrc/sysutils/xentools41/files/blktapctrl_netbsd.c?rev=1.1.1.1&content-type=text/plain&only_with_tag=MAIN

Removing blktap from the build process seems more logical than adding
a dummy library to the system in my opinion, and doesn''t affect Qemu,
since only the init_blktap function was called.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 2011-10-05 at 08:46 +0100, Roger Pau Monné
wrote:
> > Really?
> 
> NetBSD added this files to blktap lib to be able to compile it, since
> Qemu needed it:
> 
>
http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/pkgsrc/sysutils/xentools41/files/blk_netbsd.c?rev=1.1.1.1&content-type=text/plain&only_with_tag=MAIN
>
http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/pkgsrc/sysutils/xentools41/files/blktapctrl_netbsd.c?rev=1.1.1.1&content-type=text/plain&only_with_tag=MAIN
> 
> Removing blktap from the build process seems more logical than adding
> a dummy library to the system in my opinion, and doesn''t affect
Qemu,
> since only the init_blktap function was called.
I meant is the author of this patch really called "Charlie Root" with
email address "root@loki.(none)". The email address is certainly
invalid.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Sorry, I didn''t realize that, it was my first time sending patches
using git send-email, should I resend the patch?

Regards, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

NetBSD has no blktap support, so remove the use of the blktap if the OS is
NetBSD.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
---
 hw/xen_machine_pv.c |    2 +-
 xen-config-host.h   |    2 +-
 xen-hooks.mak       |    2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index 0004904..c09adbb 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -47,7 +47,7 @@ static void xen_init_pv(ram_addr_t ram_size, int vga_ram_size,
     CPUState *env;
     uint32_t domid_target;
 
-#ifndef CONFIG_STUBDOM
+#if !defined(CONFIG_STUBDOM) && !defined(__NetBSD__)
     /* Initialize tapdisk client */
     init_blktap();
 #endif
diff --git a/xen-config-host.h b/xen-config-host.h
index f50c3aa..818f25d 100644
--- a/xen-config-host.h
+++ b/xen-config-host.h
@@ -18,7 +18,7 @@ extern int domid, domid_backend;
 
 #include "xenctrl.h"
 #include "xs.h"
-#ifndef CONFIG_STUBDOM
+#if !defined(CONFIG_STUBDOM) && !defined(__NetBSD__)
 #include "blktaplib.h"
 #endif
 
diff --git a/xen-hooks.mak b/xen-hooks.mak
index 253915d..b55f45b 100644
--- a/xen-hooks.mak
+++ b/xen-hooks.mak
@@ -46,11 +46,13 @@ CONFIG_SDL CONFIG_AUDIO OBJS += xenfbfront.o
 else
+ifndef CONFIG_NetBSD
 CPPFLAGS+= -I$(XEN_ROOT)/tools/blktap/lib
 LIBS += -L$(XEN_ROOT)/tools/blktap/lib -lblktap
 OBJS += xen_blktap.o
 OBJS += tpm_tis.o
 endif
+endif
 
 ifdef CONFIG_STUBDOM
 CONFIG_PASSTHROUGH=1
-- 
1.7.6.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 2011-10-05 at 09:00 +0100, Roger Pau Monné
wrote:
> Sorry, I didn''t realize that, it was my first time sending patches
> using git send-email, should I resend the patch?
Either that or follow up to the original with the correct details for
the author.

BTW, by adding your Signed-off-by you are certifying the contribution
per the Developer''s Certificate of Origin
(http://lwn.net/Articles/437739/) which certifies that the contribution
can be submitted by you under the relevant open source license.

This particular patch is probably too trivial to be an issue but in
general you would strictly speaking need a signed-off-by from the
original author, unless you happen to work for the same institution and
have right to contribute the works of others within that organisation.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> Either that or follow up to the original with the correct details for
> the author.
>
> BTW, by adding your Signed-off-by you are certifying the contribution
> per the Developer''s Certificate of Origin
> (http://lwn.net/Articles/437739/) which certifies that the contribution
> can be submitted by you under the relevant open source license.
>
> This particular patch is probably too trivial to be an issue but in
> general you would strictly speaking need a signed-off-by from the
> original author, unless you happen to work for the same institution and
> have right to contribute the works of others within that organisation.
Sorry, I think I don''t get this right, I''m the original author
of this
patch, the two links I''ve posted contain other different patches, that
should not be added to Xen, because this patch fixes the issue using a
different approach, so I think it''s fine to include my signed-off-by.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 2011-10-05 at 11:18 +0100, Roger Pau Monné
wrote:
> > Either that or follow up to the original with the correct details for
> > the author.
> >
> > BTW, by adding your Signed-off-by you are certifying the contribution
> > per the Developer''s Certificate of Origin
> > (http://lwn.net/Articles/437739/) which certifies that the
contribution
> > can be submitted by you under the relevant open source license.
> >
> > This particular patch is probably too trivial to be an issue but in
> > general you would strictly speaking need a signed-off-by from the
> > original author, unless you happen to work for the same institution
and
> > have right to contribute the works of others within that organisation.
> 
> Sorry, I think I don''t get this right, I''m the original
author of this
> patch, the two links I''ve posted contain other different patches,
that
> should not be added to Xen, because this patch fixes the issue using a
> different approach, so I think it''s fine to include my
signed-off-by.
Ah, that''s OK then. I thought the original author was this
"Charlie
Root" guy/pseudonym, but I suppose that''s actually you and/or an
artefact of your local git repo/git send-email.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> Ah, that''s OK then. I thought the original author was this
"Charlie
> Root" guy/pseudonym, but I suppose that''s actually you and/or
an
> artefact of your local git repo/git send-email.
Sorry for the trouble, that is a problem with git, that doesn''t get
the name and email attributes right when using "git format-patch", I
think that it used the default name of the root account, which is
"Charlie", hence the "Charlie Root" name, at the end I had
to remove
the "From" fields manually, because I wasn''t able to generate
a patch
with the proper "From" field.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 2011-10-05 at 11:29 +0100, Roger Pau Monné
wrote:
> > Ah, that''s OK then. I thought the original author was this
"Charlie
> > Root" guy/pseudonym, but I suppose that''s actually you
and/or an
> > artefact of your local git repo/git send-email.
> 
> Sorry for the trouble,
No problem.
> that is a problem with git, that doesn''t get
> the name and email attributes right when using "git
format-patch", I
> think that it used the default name of the root account, which is
> "Charlie", hence the "Charlie Root" name, at the end I
had to remove
> the "From" fields manually, because I wasn''t able to
generate a patch
> with the proper "From" field.
I think the problem was probably actually a "git commit" time when the
Author field in the commit seems to have been set to "Charlie Root".
"git format-patch" just picks up on that and because it is different
to
the sender it includes a From line.

You probably need to add a ~/.gitconfig containing e.g.:
[user]
        name = Ian Campbell
        email = ian.campbell@citrix.com
so that "git commit" will pickup the right details and/or avoid
committing stuff as root.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> I think the problem was probably actually a "git commit" time
when the
> Author field in the commit seems to have been set to "Charlie
Root".
> "git format-patch" just picks up on that and because it is
different to
> the sender it includes a From line.
>
> You probably need to add a ~/.gitconfig containing e.g.:
> [user]
>        name = Ian Campbell
>        email = ian.campbell@citrix.com
> so that "git commit" will pickup the right details and/or avoid
> committing stuff as root.
You are right, when doing the commit I still haven''t set the name and
email values, I added them after, when doing the format-patch and
email. Well, since they are set now I don''t think I will have more
problems, thanks again for the tip!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

~Roger Pau Monne writes ("[Xen-devel] [PATCH] QEMU: remove blktap when
building for NetBSD"):
> NetBSD has no blktap support, so remove the use of the blktap if the OS is
NetBSD.
Applied to qemu-xen-unstable.git.  Is there a need for a similar patch
to qemu upstream ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, 2011-10-06 at 18:39 +0100, Ian Jackson wrote:
> ~Roger Pau Monne writes ("[Xen-devel] [PATCH] QEMU: remove blktap when
building for NetBSD"):
> > NetBSD has no blktap support, so remove the use of the blktap if the
OS is NetBSD.
> 
> Applied to qemu-xen-unstable.git.  Is there a need for a similar patch
> to qemu upstream ?
I don''t think upstream qemu has (or will/should have) any blktap
support.

Having said that it''s not exactly clear what, if anything, this blktap
support in qemu-xen is actually used for... It looks a bit like once
upon time it was an option to use qemu to provide the blktap (v1)
userspace backend instead of the tapdisk process (look for qemu and
use_ioemu in tools/blktap/drivers/blktapctrl.c). This functionality does
not appear to be present in blktap2.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>> Applied to qemu-xen-unstable.git.  Is there a need for a similar patch
>> to qemu upstream ?
>
> I don''t think upstream qemu has (or will/should have) any blktap
> support.
>From what I saw, there''s nothing blktap related in qemu-upstream.
> Having said that it''s not exactly clear what, if anything, this
blktap
> support in qemu-xen is actually used for... It looks a bit like once
> upon time it was an option to use qemu to provide the blktap (v1)
> userspace backend instead of the tapdisk process (look for qemu and
> use_ioemu in tools/blktap/drivers/blktapctrl.c). This functionality does
> not appear to be present in blktap2.
I''ve looked at the sources of qemu-unstable, and it seems the only
function that was called from the blktap library was init_blktap, but
no blktap backend was used anywhere in the qemu code, so I think it''s
safe to remove blktap from qemu after all.

Could this be applied to qemu-xen-4.1-testing?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel