Re: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations

2013-06-05 Thread Stefan Hajnoczi 
On Tue, May 28, 2013 at 05:27:25PM +0200, Kevin Wolf wrote:
 Pass in the BlockDriverState to the command handlers instead of using
 the global variable. This is an important step to make the commands
 usable outside of qemu-io.
 
 Signed-off-by: Kevin Wolf kw...@redhat.com
 ---
  cmd.c |   6 ++-
  cmd.h |   8 ++-
  qemu-io.c | 165 
 ++
  3 files changed, 100 insertions(+), 79 deletions(-)
 
 diff --git a/cmd.c b/cmd.c
 index 214c6f7..d501aab 100644
 --- a/cmd.c
 +++ b/cmd.c
 @@ -57,7 +57,7 @@ check_command(
   const cmdinfo_t *ci)
  {
   if (check_func)
 - return check_func(ci);
 + return check_func(qemuio_bs, ci);
   return 1;
  }
  
 @@ -103,7 +103,7 @@ command(
   return 0;
   }
   optind = 0;
 - return ct-cfunc(argc, argv);
 + return ct-cfunc(qemuio_bs, argc, argv);
  }
  
  const cmdinfo_t *
 @@ -452,6 +452,7 @@ static cmdinfo_t quit_cmd;
  /* ARGSUSED */
  static int
  quit_f(
 +BlockDriverState *bs,
   int argc,

tabs vs spaces.  I try to keep the existing style unless I decide to
reformat the entire section of code.

Not trying to start a flamewar but this file appears to use tabs and IMO
you should stick to that instead of mixing spaces :-).

   char**argv)
  {
 @@ -490,6 +491,7 @@ help_all(void)
  
  static int
  help_f(
 +BlockDriverState *bs,
   int argc,
   char**argv)
  {
 diff --git a/cmd.h b/cmd.h
 index 4dcfe88..ccf6336 100644
 --- a/cmd.h
 +++ b/cmd.h
 @@ -17,9 +17,13 @@
  #ifndef __COMMAND_H__
  #define __COMMAND_H__
  
 +#include qemu-common.h
 +
  #define CMD_FLAG_GLOBAL  ((int)0x8000)   /* don't iterate args 
 */
  
 -typedef int (*cfunc_t)(int argc, char **argv);
 +extern BlockDriverState *qemuio_bs;
 +
 +typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv);
  typedef void (*helpfunc_t)(void);
  
  typedef struct cmdinfo {
 @@ -41,7 +45,7 @@ extern int  ncmds;
  void help_init(void);
  void quit_init(void);
  
 -typedef int (*checkfunc_t)(const cmdinfo_t *ci);
 +typedef int (*checkfunc_t)(BlockDriverState *bs, const cmdinfo_t *ci);
  
  void add_command(const cmdinfo_t *ci);
  void add_user_command(char *optarg);

cmd.h does not know about the block layer.  I would use void *opaque
instead of BlockDriverState *bs.  That way the file stays generic and
can be used in other command-line tools.

Re: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations

2013-06-05 Thread Kevin Wolf 
Am 05.06.2013 um 14:28 hat Stefan Hajnoczi geschrieben:
 On Tue, May 28, 2013 at 05:27:25PM +0200, Kevin Wolf wrote:
  Pass in the BlockDriverState to the command handlers instead of using
  the global variable. This is an important step to make the commands
  usable outside of qemu-io.
  
  Signed-off-by: Kevin Wolf kw...@redhat.com
  ---
   cmd.c |   6 ++-
   cmd.h |   8 ++-
   qemu-io.c | 165 
  ++
   3 files changed, 100 insertions(+), 79 deletions(-)
  
  diff --git a/cmd.c b/cmd.c
  index 214c6f7..d501aab 100644
  --- a/cmd.c
  +++ b/cmd.c
  @@ -57,7 +57,7 @@ check_command(
  const cmdinfo_t *ci)
   {
  if (check_func)
  -   return check_func(ci);
  +   return check_func(qemuio_bs, ci);
  return 1;
   }
   
  @@ -103,7 +103,7 @@ command(
  return 0;
  }
  optind = 0;
  -   return ct-cfunc(argc, argv);
  +   return ct-cfunc(qemuio_bs, argc, argv);
   }
   
   const cmdinfo_t *
  @@ -452,6 +452,7 @@ static cmdinfo_t quit_cmd;
   /* ARGSUSED */
   static int
   quit_f(
  +BlockDriverState *bs,
  int argc,
 
 tabs vs spaces.  I try to keep the existing style unless I decide to
 reformat the entire section of code.
 
 Not trying to start a flamewar but this file appears to use tabs and IMO
 you should stick to that instead of mixing spaces :-).

Ah yes, didn't notice that.

Doesn't really matter though, at the end of the series cmd.c is gone.

  --- a/cmd.h
  +++ b/cmd.h
  @@ -17,9 +17,13 @@
   #ifndef __COMMAND_H__
   #define __COMMAND_H__
   
  +#include qemu-common.h
  +
   #define CMD_FLAG_GLOBAL((int)0x8000)   /* don't iterate args 
  */
   
  -typedef int (*cfunc_t)(int argc, char **argv);
  +extern BlockDriverState *qemuio_bs;
  +
  +typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv);
   typedef void (*helpfunc_t)(void);
   
   typedef struct cmdinfo {
  @@ -41,7 +45,7 @@ extern intncmds;
   void help_init(void);
   void quit_init(void);
   
  -typedef int (*checkfunc_t)(const cmdinfo_t *ci);
  +typedef int (*checkfunc_t)(BlockDriverState *bs, const cmdinfo_t *ci);
   
   void add_command(const cmdinfo_t *ci);
   void add_user_command(char *optarg);
 
 cmd.h does not know about the block layer.  I would use void *opaque
 instead of BlockDriverState *bs.  That way the file stays generic and
 can be used in other command-line tools.

Do you plan to use this in different context? Because this series is
exactly the opposite of keeping it generic. It moves everything directly
into qemu-io.

Kevin

Re: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations

2013-05-28 Thread Kevin Wolf 
Am 28.05.2013 um 17:27 hat Kevin Wolf geschrieben:
 Pass in the BlockDriverState to the command handlers instead of using
 the global variable. This is an important step to make the commands
 usable outside of qemu-io.
 
 Signed-off-by: Kevin Wolf kw...@redhat.com

 @@ -1793,7 +1808,7 @@ static const cmdinfo_t abort_cmd = {
 .oneline= simulate a program crash using abort(3),
  };
  
 -static int close_f(int argc, char **argv)
 +static int close_f(BlockDriverState *bs, int argc, char **argv)
  {
  bdrv_delete(bs);
  bs = NULL;

Here I missed the following hunk that I've applied locally now:

--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1811,7 +1811,7 @@ static const cmdinfo_t abort_cmd = {
 static int close_f(BlockDriverState *bs, int argc, char **argv)
 {
 bdrv_delete(bs);
-bs = NULL;
+qemuio_bs = NULL;
 return 0;
 }

[Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations

2013-05-28 Thread Kevin Wolf 
Pass in the BlockDriverState to the command handlers instead of using
the global variable. This is an important step to make the commands
usable outside of qemu-io.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 cmd.c |   6 ++-
 cmd.h |   8 ++-
 qemu-io.c | 165 ++
 3 files changed, 100 insertions(+), 79 deletions(-)

diff --git a/cmd.c b/cmd.c
index 214c6f7..d501aab 100644
--- a/cmd.c
+++ b/cmd.c
@@ -57,7 +57,7 @@ check_command(
const cmdinfo_t *ci)
 {
if (check_func)
-   return check_func(ci);
+   return check_func(qemuio_bs, ci);
return 1;
 }
 
@@ -103,7 +103,7 @@ command(
return 0;
}
optind = 0;
-   return ct-cfunc(argc, argv);
+   return ct-cfunc(qemuio_bs, argc, argv);
 }
 
 const cmdinfo_t *
@@ -452,6 +452,7 @@ static cmdinfo_t quit_cmd;
 /* ARGSUSED */
 static int
 quit_f(
+BlockDriverState *bs,
int argc,
char**argv)
 {
@@ -490,6 +491,7 @@ help_all(void)
 
 static int
 help_f(
+BlockDriverState *bs,
int argc,
char**argv)
 {
diff --git a/cmd.h b/cmd.h
index 4dcfe88..ccf6336 100644
--- a/cmd.h
+++ b/cmd.h
@@ -17,9 +17,13 @@
 #ifndef __COMMAND_H__
 #define __COMMAND_H__
 
+#include qemu-common.h
+
 #define CMD_FLAG_GLOBAL((int)0x8000)   /* don't iterate args 
*/
 
-typedef int (*cfunc_t)(int argc, char **argv);
+extern BlockDriverState *qemuio_bs;
+
+typedef int (*cfunc_t)(BlockDriverState *bs, int argc, char **argv);
 typedef void (*helpfunc_t)(void);
 
 typedef struct cmdinfo {
@@ -41,7 +45,7 @@ extern intncmds;
 void help_init(void);
 void quit_init(void);
 
-typedef int (*checkfunc_t)(const cmdinfo_t *ci);
+typedef int (*checkfunc_t)(BlockDriverState *bs, const cmdinfo_t *ci);
 
 void add_command(const cmdinfo_t *ci);
 void add_user_command(char *optarg);
diff --git a/qemu-io.c b/qemu-io.c
index b4f56fc..3abf9ff 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -25,8 +25,8 @@
 #define CMD_NOFILE_OK   0x01
 
 char *progname;
-static BlockDriverState *bs;
 
+BlockDriverState *qemuio_bs;
 static int misalign;
 
 static int64_t cvtnum(const char *s)
@@ -63,7 +63,7 @@ static int parse_pattern(const char *arg)
  */
 
 #define MISALIGN_OFFSET 16
-static void *qemu_io_alloc(size_t len, int pattern)
+static void *qemu_io_alloc(BlockDriverState *bs, size_t len, int pattern)
 {
 void *buf;
 
@@ -136,7 +136,8 @@ static void print_report(const char *op, struct timeval *t, 
int64_t offset,
  * vector matching it.
  */
 static void *
-create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, int pattern)
+create_iovec(BlockDriverState *bs, QEMUIOVector *qiov, char **argv, int nr_iov,
+ int pattern)
 {
 size_t *sizes = g_new0(size_t, nr_iov);
 size_t count = 0;
@@ -172,7 +173,7 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov, 
int pattern)
 
 qemu_iovec_init(qiov, nr_iov);
 
-buf = p = qemu_io_alloc(count, pattern);
+buf = p = qemu_io_alloc(bs, count, pattern);
 
 for (i = 0; i  nr_iov; i++) {
 qemu_iovec_add(qiov, p, sizes[i]);
@@ -184,7 +185,8 @@ fail:
 return buf;
 }
 
-static int do_read(char *buf, int64_t offset, int count, int *total)
+static int do_read(BlockDriverState *bs, char *buf, int64_t offset, int count,
+   int *total)
 {
 int ret;
 
@@ -196,7 +198,8 @@ static int do_read(char *buf, int64_t offset, int count, 
int *total)
 return 1;
 }
 
-static int do_write(char *buf, int64_t offset, int count, int *total)
+static int do_write(BlockDriverState *bs, char *buf, int64_t offset, int count,
+int *total)
 {
 int ret;
 
@@ -208,7 +211,8 @@ static int do_write(char *buf, int64_t offset, int count, 
int *total)
 return 1;
 }
 
-static int do_pread(char *buf, int64_t offset, int count, int *total)
+static int do_pread(BlockDriverState *bs, char *buf, int64_t offset, int count,
+int *total)
 {
 *total = bdrv_pread(bs, offset, (uint8_t *)buf, count);
 if (*total  0) {
@@ -217,7 +221,8 @@ static int do_pread(char *buf, int64_t offset, int count, 
int *total)
 return 1;
 }
 
-static int do_pwrite(char *buf, int64_t offset, int count, int *total)
+static int do_pwrite(BlockDriverState *bs, char *buf, int64_t offset, int 
count,
+ int *total)
 {
 *total = bdrv_pwrite(bs, offset, (uint8_t *)buf, count);
 if (*total  0) {
@@ -227,6 +232,7 @@ static int do_pwrite(char *buf, int64_t offset, int count, 
int *total)
 }
 
 typedef struct {
+BlockDriverState *bs;
 int64_t offset;
 int count;
 int *total;
@@ -238,7 +244,7 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
 {
 CoWriteZeroes *data = opaque;
 
-data-ret = bdrv_co_write_zeroes(bs, data-offset / BDRV_SECTOR_SIZE,
+data-ret = bdrv_co_write_zeroes(data-bs, data-offset / BDRV_SECTOR_SIZE,

Re: [Qemu-devel] [PATCH 05/16] qemu-io: Don't use global bs in command implementations

2013-05-28 Thread Eric Blake 
On 05/28/2013 09:37 AM, Kevin Wolf wrote:
 Am 28.05.2013 um 17:27 hat Kevin Wolf geschrieben:
 Pass in the BlockDriverState to the command handlers instead of using
 the global variable. This is an important step to make the commands
 usable outside of qemu-io.

 Signed-off-by: Kevin Wolf kw...@redhat.com
 
 @@ -1793,7 +1808,7 @@ static const cmdinfo_t abort_cmd = {
 .oneline= simulate a program crash using abort(3),
  };
  
 -static int close_f(int argc, char **argv)
 +static int close_f(BlockDriverState *bs, int argc, char **argv)
  {
  bdrv_delete(bs);
  bs = NULL;
 
 Here I missed the following hunk that I've applied locally now:
 
 --- a/qemu-io.c
 +++ b/qemu-io.c
 @@ -1811,7 +1811,7 @@ static const cmdinfo_t abort_cmd = {
  static int close_f(BlockDriverState *bs, int argc, char **argv)
  {
  bdrv_delete(bs);
 -bs = NULL;
 +qemuio_bs = NULL;

Reviewed-by: Eric Blake ebl...@redhat.com

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature