qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/3] Block patches
@ 2013-07-05 11:32 Stefan Hajnoczi
  2013-07-05 11:32 ` [Qemu-devel] [PATCH 1/3] vmdk: Implement .bdrv_has_zero_init Stefan Hajnoczi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2013-07-05 11:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Stefan Hajnoczi

The following changes since commit ab8bf29078e0ab8347e2ff8b4e5542f7a0c751cf:

  Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-07-03 08:37:00 -0500)

are available in the git repository at:


  git://github.com/stefanha/qemu.git block

for you to fetch changes up to 58fda173e1156d24e5ff62361774715152188a07:

  block: fix bdrv_flush() ordering in bdrv_close() (2013-07-05 10:52:23 +0200)

----------------------------------------------------------------
Fam Zheng (2):
      vmdk: Implement .bdrv_has_zero_init
      curl: refuse to open URL from HTTP server without range support

Stefan Hajnoczi (1):
      block: fix bdrv_flush() ordering in bdrv_close()

 block.c      |  5 +++--
 block/curl.c | 24 ++++++++++++++++++------
 block/vmdk.c | 48 +++++++++++++++++++++++++++++++++---------------
 3 files changed, 54 insertions(+), 23 deletions(-)

-- 
1.8.1.4

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

* [Qemu-devel] [PATCH 1/3] vmdk: Implement .bdrv_has_zero_init
  2013-07-05 11:32 [Qemu-devel] [PULL 0/3] Block patches Stefan Hajnoczi
@ 2013-07-05 11:32 ` Stefan Hajnoczi
  2013-07-05 11:32 ` [Qemu-devel] [PATCH 2/3] curl: refuse to open URL from HTTP server without range support Stefan Hajnoczi
  2013-07-05 11:32 ` [Qemu-devel] [PATCH 3/3] block: fix bdrv_flush() ordering in bdrv_close() Stefan Hajnoczi
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2013-07-05 11:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Fam Zheng, Stefan Hajnoczi

From: Fam Zheng <famz@redhat.com>

Depending on the subformat, has_zero_init queries underlying storage for
flat extent. If it has a flat extent and its underlying storage doesn't
have zero init, return 0. Otherwise return 1.

Aligns the operator assignments.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vmdk.c | 48 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index a28fb5e..3756333 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1724,6 +1724,23 @@ static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs)
     return ret;
 }
 
+static int vmdk_has_zero_init(BlockDriverState *bs)
+{
+    int i;
+    BDRVVmdkState *s = bs->opaque;
+
+    /* If has a flat extent and its underlying storage doesn't have zero init,
+     * return 0. */
+    for (i = 0; i < s->num_extents; i++) {
+        if (s->extents[i].flat) {
+            if (!bdrv_has_zero_init(s->extents[i].file)) {
+                return 0;
+            }
+        }
+    }
+    return 1;
+}
+
 static QEMUOptionParameter vmdk_create_options[] = {
     {
         .name = BLOCK_OPT_SIZE,
@@ -1762,21 +1779,22 @@ static QEMUOptionParameter vmdk_create_options[] = {
 };
 
 static BlockDriver bdrv_vmdk = {
-    .format_name    = "vmdk",
-    .instance_size  = sizeof(BDRVVmdkState),
-    .bdrv_probe     = vmdk_probe,
-    .bdrv_open      = vmdk_open,
-    .bdrv_reopen_prepare = vmdk_reopen_prepare,
-    .bdrv_read      = vmdk_co_read,
-    .bdrv_write     = vmdk_co_write,
-    .bdrv_co_write_zeroes = vmdk_co_write_zeroes,
-    .bdrv_close     = vmdk_close,
-    .bdrv_create    = vmdk_create,
-    .bdrv_co_flush_to_disk  = vmdk_co_flush,
-    .bdrv_co_is_allocated   = vmdk_co_is_allocated,
-    .bdrv_get_allocated_file_size  = vmdk_get_allocated_file_size,
-
-    .create_options = vmdk_create_options,
+    .format_name                  = "vmdk",
+    .instance_size                = sizeof(BDRVVmdkState),
+    .bdrv_probe                   = vmdk_probe,
+    .bdrv_open                    = vmdk_open,
+    .bdrv_reopen_prepare          = vmdk_reopen_prepare,
+    .bdrv_read                    = vmdk_co_read,
+    .bdrv_write                   = vmdk_co_write,
+    .bdrv_co_write_zeroes         = vmdk_co_write_zeroes,
+    .bdrv_close                   = vmdk_close,
+    .bdrv_create                  = vmdk_create,
+    .bdrv_co_flush_to_disk        = vmdk_co_flush,
+    .bdrv_co_is_allocated         = vmdk_co_is_allocated,
+    .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
+    .bdrv_has_zero_init           = vmdk_has_zero_init,
+
+    .create_options               = vmdk_create_options,
 };
 
 static void bdrv_vmdk_init(void)
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH 2/3] curl: refuse to open URL from HTTP server without range support
  2013-07-05 11:32 [Qemu-devel] [PULL 0/3] Block patches Stefan Hajnoczi
  2013-07-05 11:32 ` [Qemu-devel] [PATCH 1/3] vmdk: Implement .bdrv_has_zero_init Stefan Hajnoczi
@ 2013-07-05 11:32 ` Stefan Hajnoczi
  2013-07-05 11:32 ` [Qemu-devel] [PATCH 3/3] block: fix bdrv_flush() ordering in bdrv_close() Stefan Hajnoczi
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2013-07-05 11:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Fam Zheng, Stefan Hajnoczi

From: Fam Zheng <famz@redhat.com>

CURL driver requests partial data from server on guest IO req. For HTTP
and HTTPS, it uses "Range: ***" in requests, and this will not work if
server not accepting range. This patch does this check when open.

 * Removed curl_size_cb, which is not used: On one hand it's registered to
   libcurl as CURLOPT_WRITEFUNCTION, instead of CURLOPT_HEADERFUNCTION,
   which will get called with *data*, not *header*. On the other hand the
   s->len is assigned unconditionally later.

   In this gone function, the sscanf for "Content-Length: %zd", on
   (void *)ptr, which is not guaranteed to be zero-terminated, is
   potentially a security bug. So this patch fixes it as a side-effect. The
   bug is reported as: https://bugs.launchpad.net/qemu/+bug/1188943
   (Note the bug is marked "private" so you might not be able to see it)

 * Introduced curl_header_cb, which is used to parse header and mark the
   server as accepting range if "Accept-Ranges: bytes" line is seen from
   response header. If protocol is HTTP or HTTPS, but server response has
   no not this support, refuse to open this URL.

Note that python builtin module SimpleHTTPServer is an example of not
supporting range, if you need to test this driver, get a better server
or use internet URLs.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/curl.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 6af8cb7..82d39ff 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -81,6 +81,7 @@ typedef struct BDRVCURLState {
     CURLState states[CURL_NUM_STATES];
     char *url;
     size_t readahead_size;
+    bool accept_range;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
@@ -110,14 +111,15 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
     return 0;
 }
 
-static size_t curl_size_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
+static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
-    CURLState *s = ((CURLState*)opaque);
+    BDRVCURLState *s = opaque;
     size_t realsize = size * nmemb;
-    size_t fsize;
+    const char *accept_line = "Accept-Ranges: bytes";
 
-    if(sscanf(ptr, "Content-Length: %zd", &fsize) == 1) {
-        s->s->len = fsize;
+    if (realsize >= strlen(accept_line)
+        && strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) {
+        s->accept_range = true;
     }
 
     return realsize;
@@ -447,8 +449,11 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
 
     // Get file size
 
+    s->accept_range = false;
     curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1);
-    curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, (void *)curl_size_cb);
+    curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION,
+                     curl_header_cb);
+    curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
     if (curl_easy_perform(state->curl))
         goto out;
     curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d);
@@ -456,6 +461,13 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
         s->len = (size_t)d;
     else if(!s->len)
         goto out;
+    if ((!strncasecmp(s->url, "http://", strlen("http://"))
+        || !strncasecmp(s->url, "https://", strlen("https://")))
+        && !s->accept_range) {
+        pstrcpy(state->errmsg, CURL_ERROR_SIZE,
+                "Server does not support 'range' (byte ranges).");
+        goto out;
+    }
     DPRINTF("CURL: Size = %zd\n", s->len);
 
     curl_clean_state(state);
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH 3/3] block: fix bdrv_flush() ordering in bdrv_close()
  2013-07-05 11:32 [Qemu-devel] [PULL 0/3] Block patches Stefan Hajnoczi
  2013-07-05 11:32 ` [Qemu-devel] [PATCH 1/3] vmdk: Implement .bdrv_has_zero_init Stefan Hajnoczi
  2013-07-05 11:32 ` [Qemu-devel] [PATCH 2/3] curl: refuse to open URL from HTTP server without range support Stefan Hajnoczi
@ 2013-07-05 11:32 ` Stefan Hajnoczi
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2013-07-05 11:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, qemu-stable, Stefan Hajnoczi

Since 80ccf93b we flush the block device during close.  The
bdrv_drain_all() call should come before bdrv_flush() to ensure guest
write requests have completed.  Otherwise we may miss pending writes
when flushing.

Call bdrv_drain_all() again for safety as the final step after
bdrv_flush().  This should not be necessary but we can be paranoid here
in case bdrv_flush() left I/O pending.

Cc: qemu-stable@nongnu.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 6c493ad..183fec8 100644
--- a/block.c
+++ b/block.c
@@ -1358,11 +1358,12 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
 
 void bdrv_close(BlockDriverState *bs)
 {
-    bdrv_flush(bs);
     if (bs->job) {
         block_job_cancel_sync(bs->job);
     }
-    bdrv_drain_all();
+    bdrv_drain_all(); /* complete I/O */
+    bdrv_flush(bs);
+    bdrv_drain_all(); /* in case flush left pending I/O */
     notifier_list_notify(&bs->close_notifiers, bs);
 
     if (bs->drv) {
-- 
1.8.1.4

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

end of thread, other threads:[~2013-07-05 11:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-05 11:32 [Qemu-devel] [PULL 0/3] Block patches Stefan Hajnoczi
2013-07-05 11:32 ` [Qemu-devel] [PATCH 1/3] vmdk: Implement .bdrv_has_zero_init Stefan Hajnoczi
2013-07-05 11:32 ` [Qemu-devel] [PATCH 2/3] curl: refuse to open URL from HTTP server without range support Stefan Hajnoczi
2013-07-05 11:32 ` [Qemu-devel] [PATCH 3/3] block: fix bdrv_flush() ordering in bdrv_close() Stefan Hajnoczi

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).