qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/5] Fast Virtual Disk (FVD) Proposal Part 1


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 1/5] Fast Virtual Disk (FVD) Proposal Part 1
Date: Fri, 21 Jan 2011 16:41:00 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Lightning/1.0b1 Thunderbird/3.0.10

On 01/19/2011 04:04 PM, Chunqiang Tang wrote:
Part 1 of the block device driver for the proposed FVD image format.
Multiple patches are used in order to manage the size of each patch.
This patch includes existing files that are modified by FVD.

See the related discussions at
http://lists.gnu.org/archive/html/qemu-devel/2011-01/msg00426.html .

Signed-off-by: Chunqiang Tang<address@hidden>
---
  Makefile         |   10 +++++---
  Makefile.objs    |    1 +
  block.c          |   12 +++++-----
  block_int.h      |    5 ++-
  configure        |    2 +-
  qemu-img-cmds.hx |    6 +++++
  qemu-img.c       |   62 ++++++++++++++++++++++++++++++++++++++++++++---------
  qemu-io.c        |    3 ++
  qemu-option.c    |    4 +++
  qemu-tool.c      |   36 -------------------------------
  10 files changed, 81 insertions(+), 60 deletions(-)

diff --git a/Makefile b/Makefile
index 6d601ee..da4d777 100644
--- a/Makefile
+++ b/Makefile
@@ -151,13 +151,15 @@ version-obj-$(CONFIG_WIN32) += version.o
  ######################################################################

  qemu-img.o: qemu-img-cmds.h
-qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o: $(GENERATED_HEADERS)
+qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o qemu-test.o: 
$(GENERATED_HEADERS)

-qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
+qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-tool-time.o qemu-error.o 
$(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o

-qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
+qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-tool-time.o qemu-error.o 
$(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o

-qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
+qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-tool-time.o qemu-error.o 
$(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
+
+qemu-test$(EXESUF): qemu-test.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o

  qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
        $(call quiet-command,sh $(SRC_PATH)/hxtool -h<  $<  >  $@,"  GEN   $@")
diff --git a/Makefile.objs b/Makefile.objs
index c3e52c5..c0c1155 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -23,6 +23,7 @@ block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o 
qcow2-snapshot.o
  block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
  block-nested-y += qed-check.o
  block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
+block-nested-y += blksim.o fvd.o
  block-nested-$(CONFIG_WIN32) += raw-win32.o
  block-nested-$(CONFIG_POSIX) += raw-posix.o
  block-nested-$(CONFIG_CURL) += curl.o
diff --git a/block.c b/block.c
index ff2795b..856bb1a 100644
--- a/block.c
+++ b/block.c
@@ -58,7 +58,7 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t 
sector_num,
  static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
                           const uint8_t *buf, int nb_sectors);

-static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
+QTAILQ_HEAD(, BlockDriverState) bdrv_states =
      QTAILQ_HEAD_INITIALIZER(bdrv_states);

This looks suspicious and indicates your doing something bad.


  static QLIST_HEAD(, BlockDriver) bdrv_drivers =
@@ -768,7 +768,7 @@ int bdrv_commit(BlockDriverState *bs)

      if (!drv)
          return -ENOMEDIUM;
-
+
      if (!bs->backing_hd) {
          return -ENOTSUP;
      }
@@ -1538,7 +1538,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors)
   * 'nb_sectors' is the max value 'pnum' should be set to.
   */
  int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int 
nb_sectors,
-       int *pnum)
+        int *pnum)
  {
      int64_t n;
      if (!bs->drv->bdrv_is_allocated) {
@@ -2050,9 +2050,9 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, 
int64_t sector_num,
                                cb, opaque);

      if (ret) {
-       /* Update stats even though technically transfer has not happened. */
-       bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
-       bs->rd_ops ++;
+        /* Update stats even though technically transfer has not happened. */
+        bs->rd_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
+        bs->rd_ops ++;
      }

      return ret;
diff --git a/block_int.h b/block_int.h
index 12663e8..2343d07 100644
--- a/block_int.h
+++ b/block_int.h
@@ -28,8 +28,8 @@
  #include "qemu-option.h"
  #include "qemu-queue.h"

-#define BLOCK_FLAG_ENCRYPT     1
-#define BLOCK_FLAG_COMPAT6     4
+#define BLOCK_FLAG_ENCRYPT        1
+#define BLOCK_FLAG_COMPAT6        4

  #define BLOCK_OPT_SIZE          "size"
  #define BLOCK_OPT_ENCRYPT       "encryption"
@@ -98,6 +98,7 @@ struct BlockDriver {
      int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
                                    const char *snapshot_name);
      int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
+    int (*bdrv_update)(BlockDriverState *bs, int argc, char **argv);

argc/argv is an awful interface because the semantics end up varying widely. If we want to support changing disk format parameters, we should use a structured option format so we can ensure it's exposed to the user in a consistent way. IOW, a size is always parsed as <integer>[SUFFIX] and not 8 different variations of that theme.

      bs = qemu_mallocz(bs_n * sizeof(BlockDriverState *));

      total_sectors = 0;
@@ -865,7 +865,7 @@ static int img_convert(int argc, char **argv)
                     assume that sectors which are unallocated in the input 
image
                     are present in both the output's and input's base images 
(no
                     need to copy them). */
-                if (out_baseimg) {
+                if (out_baseimg || bs[bs_i]->backing_file[0]==0) {

This looks like a bug fix of some sort and should be it's own patch with an explanation.

                      if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
                                             n,&n1)) {
                          sector_num += n1;
@@ -941,10 +941,10 @@ static int64_t get_allocated_file_size(const char 
*filename)
      /* WinNT support GetCompressedFileSize to determine allocate size */
      get_compressed = (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"), 
"GetCompressedFileSizeA");
      if (get_compressed) {
-       DWORD high, low;
-       low = get_compressed(filename,&high);
-       if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR)
-           return (((int64_t) high)<<  32) + low;
+            DWORD high, low;
+            low = get_compressed(filename,&high);
+            if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR)
+            return (((int64_t) high)<<  32) + low;
      }

      if (_stati64(filename,&st)<  0)
@@ -1036,11 +1036,6 @@ static int img_info(int argc, char **argv)
      if (bdrv_is_encrypted(bs)) {
          printf("encrypted: yes\n");
      }
-    if (bdrv_get_info(bs,&bdi)>= 0) {
-        if (bdi.cluster_size != 0) {
-            printf("cluster_size: %d\n", bdi.cluster_size);
-        }
-    }
      bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
      if (backing_filename[0] != '\0') {
          path_combine(backing_filename2, sizeof(backing_filename2),
@@ -1049,11 +1044,56 @@ static int img_info(int argc, char **argv)
                 backing_filename,
                 backing_filename2);
      }
+    if (bdrv_get_info(bs,&bdi)>= 0) {
+        if (bdi.cluster_size != 0)
+            printf("cluster_size: %d\n", bdi.cluster_size);
+    }
      dump_snapshots(bs);
      bdrv_delete(bs);
      return 0;
  }

+static int img_update(int argc, char **argv)
+{
+    int c;
+    const char *filename, *fmt;
+    BlockDriverState *bs;
+
+    fmt = NULL;
+    for(;;) {
+        c = getopt(argc, argv, "f:h");
+        if (c == -1)
+            break;
+        switch(c) {
+        case 'h':
+            help();
+            break;
+        case 'f':
+            fmt = optarg;
+            break;
+        }
+    }
+    if (optind>= argc)
+        help();
+    filename = argv[optind++];
+
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING | 
BDRV_O_RDWR);
+    if (!bs) {
+        return 1;
+    }
+
+    if (bs->drv->bdrv_update==NULL) {
+        char fmt_name[128];
+        bdrv_get_format(bs, fmt_name, sizeof(fmt_name));
+        error_report ("the 'update' command is not supported for the '%s' image 
format.", fmt_name);
+    }
+
+    bs->drv->bdrv_update(bs, argc-optind,&argv[optind]);
+
+    bdrv_delete(bs);
+    return 0;
+}
+
  #define SNAPSHOT_LIST   1
  #define SNAPSHOT_CREATE 2
  #define SNAPSHOT_APPLY  3
diff --git a/qemu-io.c b/qemu-io.c
index 5b24c5e..c32f8d4 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1701,6 +1701,8 @@ init_check_command(
        return 1;
  }

+#include "qemu-io-sim.c"
+

1) I don't see qemu-io-sim.c in this patch which means this breaks the build

2) Including C files is evil.

  static void usage(const char *name)
  {
        printf(
@@ -1807,6 +1809,7 @@ int main(int argc, char **argv)
        add_command(&discard_cmd);
        add_command(&alloc_cmd);
        add_command(&map_cmd);
+        add_command(&sim_cmd);

        add_args_command(init_args_command);
        add_check_command(init_check_command);
diff --git a/qemu-option.c b/qemu-option.c
index 65db542..10ef45f 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -289,6 +289,10 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
              return -1;
          break;

+    case OPT_NUMBER:
+        list->value.n = atoi (value);
+        break;
+
      default:
          fprintf(stderr, "Bug: Option '%s' has an unknown type\n", name);
          return -1;
diff --git a/qemu-tool.c b/qemu-tool.c
index 392e1c9..fdcb2f8 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -23,12 +23,6 @@ QEMUClock *rt_clock;

  FILE *logfile;

-struct QEMUBH
-{
-    QEMUBHFunc *cb;
-    void *opaque;
-};
-
  void qemu_service_io(void)
  {
  }
@@ -73,36 +67,6 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
  {
  }

-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
-{
-    QEMUBH *bh;
-
-    bh = qemu_malloc(sizeof(*bh));
-    bh->cb = cb;
-    bh->opaque = opaque;
-
-    return bh;
-}
-
-int qemu_bh_poll(void)
-{
-    return 0;
-}
-
-void qemu_bh_schedule(QEMUBH *bh)
-{
-    bh->cb(bh->opaque);
-}
-
-void qemu_bh_cancel(QEMUBH *bh)
-{
-}
-
-void qemu_bh_delete(QEMUBH *bh)
-{
-    qemu_free(bh);
-}
-
  int qemu_set_fd_handler2(int fd,
                           IOCanReadHandler *fd_read_poll,
                           IOHandler *fd_read,

These functions surely cannot just be deleted like this.

Regards,

Anthony Liguori



reply via email to

[Prev in Thread] Current Thread [Next in Thread]