1. 30 3月, 2016 7 次提交
    • K
      block: Remove BDRV_O_CACHE_WB · 61de4c68
      Kevin Wolf 提交于
      The previous patches have successively made blk->enable_write_cache the
      true source for the information whether a writethrough mode must be
      implemented. The corresponding BDRV_O_CACHE_WB is only useless baggage
      we're carrying around, so now's the time to remove it.
      
      At the same time, we remove the 'cache.writeback' option parsing on the
      BDS level as the only effect was setting the BDRV_O_CACHE_WB flag.
      
      This change requires test cases that explicitly enabled the option to
      drop it. Other than that and the change of the error message when
      writethrough is enabled on the BDS level (from "Can't set writethrough
      mode" to "doesn't support the option"), there should be no change in
      behaviour.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      61de4c68
    • K
      block: Use bdrv_parse_cache_mode() in drive_init() · 04feb4a5
      Kevin Wolf 提交于
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      04feb4a5
    • K
      block: Always set writeback mode in blk_new_open() · 72e775c7
      Kevin Wolf 提交于
      All callers of blk_new_open() either don't rely on the WCE bit set after
      blk_new_open() because they explicitly set it anyway, or they pass
      BDRV_O_CACHE_WB unconditionally.
      
      This patch changes blk_new_open() so that it always enables writeback
      mode and asserts that BDRV_O_CACHE_WB is clear. For those callers that
      used to pass BDRV_O_CACHE_WB unconditionally, the flag is removed now.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      72e775c7
    • K
      e4b24b49
    • K
      block: Reject writethrough mode except at the root · 73ac451f
      Kevin Wolf 提交于
      Writethrough mode is going to become a BlockBackend feature rather than
      a BDS one, so forbid it in places where we won't be able to support it
      when the code finally matches the envisioned design.
      
      We only allowed setting the cache mode of non-root nodes after the 2.5
      release, so we're still free to make this change.
      
      The target of block jobs is now always opened in a writeback mode
      because it doesn't have a BlockBackend attached. This makes more sense
      anyway because block jobs know when to flush. If the graph is modified
      on job completion, the original cache mode moves to the new root, so
      for the guest device writethough always stays enabled if it was
      configured this way.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      73ac451f
    • K
      block: Remove copy-on-read from bdrv_move_feature_fields() · 4c844983
      Kevin Wolf 提交于
      Ever since we first introduced bdrv_append() in commit 8802d1fd ('qapi:
      Introduce blockdev-group-snapshot-sync command'), the copy-on-read flag
      was moved to the new top layer when taking a snapshot. The only problem
      is that it doesn't make a whole lot of sense.
      
      The use case for manually enabled CoR is to avoid reading data twice
      from a slow remote image, so we want to save it to a local overlay, say
      an ISO image accessed via HTTP to a local qcow2 overlay. When taking a
      snapshot, we end up with a backing chain like this:
      
          http <- local.qcow2 <- snap_overlay.qcow2
      
      There is no point in doing CoR from local.qcow2 into snap_overlay.qcow2,
      we just want to keep copying data from the remote source into
      local.qcow2.
      
      The other use case of CoR is in the context of streaming, which isn't
      very interesting for bdrv_move_feature_fields() because op blockers
      prevent this combination.
      
      This patch makes the copy-on-read flag stay on the image for which it
      was originally set and prevents it from being propagated to the new
      overlay. It is no longer intended to move CoR to the BlockBackend level.
      In order for this to make sense, we also need to keep the respective
      image read-write.
      
      As a side effect of these changes, creating a live snapshot image (as
      opposed to using an existing externally created one) on top of a COR
      block device works now. It used to fail because it tried to open its
      backing file both read-only and with COR.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      4c844983
    • K
      block: Remove bdrv_make_anon() · 63eaaae0
      Kevin Wolf 提交于
      The call in hmp_drive_del() is dead code because blk_remove_bs() is
      called a few lines above. The only other remaining user is
      bdrv_delete(), which only abuses bdrv_make_anon() to remove it from the
      named nodes list. This path inlines the list entry removal into
      bdrv_delete() and removes bdrv_make_anon().
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      63eaaae0
  2. 23 3月, 2016 1 次提交
  3. 18 3月, 2016 1 次提交
    • E
      qapi: Don't special-case simple union wrappers · 32bafa8f
      Eric Blake 提交于
      Simple unions were carrying a special case that hid their 'data'
      QMP member from the resulting C struct, via the hack method
      QAPISchemaObjectTypeVariant.simple_union_type().  But by using
      the work we started by unboxing flat union and alternate
      branches, coupled with the ability to visit the members of an
      implicit type, we can now expose the simple union's implicit
      type in qapi-types.h:
      
      | struct q_obj_ImageInfoSpecificQCow2_wrapper {
      |     ImageInfoSpecificQCow2 *data;
      | };
      |
      | struct q_obj_ImageInfoSpecificVmdk_wrapper {
      |     ImageInfoSpecificVmdk *data;
      | };
      ...
      | struct ImageInfoSpecific {
      |     ImageInfoSpecificKind type;
      |     union { /* union tag is @type */
      |         void *data;
      |-        ImageInfoSpecificQCow2 *qcow2;
      |-        ImageInfoSpecificVmdk *vmdk;
      |+        q_obj_ImageInfoSpecificQCow2_wrapper qcow2;
      |+        q_obj_ImageInfoSpecificVmdk_wrapper vmdk;
      |     } u;
      | };
      
      Doing this removes asymmetry between QAPI's QMP side and its
      C side (both sides now expose 'data'), and means that the
      treatment of a simple union as sugar for a flat union is now
      equivalent in both languages (previously the two approaches used
      a different layer of dereferencing, where the simple union could
      be converted to a flat union with equivalent C layout but
      different {} on the wire, or to an equivalent QMP wire form
      but with different C representation).  Using the implicit type
      also lets us get rid of the simple_union_type() hack.
      
      Of course, now all clients of simple unions have to adjust from
      using su->u.member to using su->u.member.data; while this touches
      a number of files in the tree, some earlier cleanup patches
      helped minimize the change to the initialization of a temporary
      variable rather than every single member access.  The generated
      qapi-visit.c code is also affected by the layout change:
      
      |@@ -7393,10 +7393,10 @@ void visit_type_ImageInfoSpecific_member
      |     }
      |     switch (obj->type) {
      |     case IMAGE_INFO_SPECIFIC_KIND_QCOW2:
      |-        visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2, &err);
      |+        visit_type_q_obj_ImageInfoSpecificQCow2_wrapper_members(v, &obj->u.qcow2, &err);
      |         break;
      |     case IMAGE_INFO_SPECIFIC_KIND_VMDK:
      |-        visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk, &err);
      |+        visit_type_q_obj_ImageInfoSpecificVmdk_wrapper_members(v, &obj->u.vmdk, &err);
      |         break;
      |     default:
      |         abort();
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1458254921-17042-13-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      32bafa8f
  4. 17 3月, 2016 7 次提交
  5. 14 3月, 2016 5 次提交
    • K
      hmp: Extend drive_del to delete nodes without BB · 2073d410
      Kevin Wolf 提交于
      Now that we can use drive_add to create new nodes without a BB, we also
      want to be able to delete such nodes again.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      2073d410
    • K
      hmp: 'drive_add -n' for creating a node without BB · abb21ac3
      Kevin Wolf 提交于
      This patch adds an option to the drive_add HMP command to create only a
      BlockDriverState without a BlockBackend on top.
      
      The motivation for this is that libvirt needs to specify options to a
      migration target (specifically, detect-zeroes). drive-mirror doesn't
      allow specifying options, and the proper way to do this is to create the
      target BDS separately with blockdev-add (where you can specify options)
      and then use blockdev-mirror to that BDS.
      
      However, libvirt can't use blockdev-add as long as it is still
      experimental, and we're expecting that it will still take some time, so
      we need to resort to drive_add.
      
      The problem with drive_add is that so far it always created a BB, and
      BDSes with a BB can't be used as a mirroring target as long as we don't
      support multiple BBs per BDS - and while we're working towards that
      goal, it's another thing that will still take some time.
      
      So to achieve the goal, the simplest solution to provide the
      functionality now without adding one-off options to the mirror QMP
      commands is to extend drive_add to create nodes without BBs.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      abb21ac3
    • K
      block: Fix cache mode defaults in bds_tree_init() · a81d6164
      Kevin Wolf 提交于
      Without setting explicit defaults in the options, blockdev-add without
      an ID ended up defaulting to writethrough. It should be writeback as
      documented.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      a81d6164
    • K
      block: Fix snapshot=on cache modes · 73176bee
      Kevin Wolf 提交于
      Since commit 91a097e7, we end up with a somewhat weird cache mode
      configuration with snapshot=on: The commit broke the cache mode
      inheritance for the snapshot overlay so that it is opened as
      writethrough instead of unsafe now. The following bdrv_append() call to
      put it on top of the tree swaps the WCE flag with the snapshot's backing
      file (i.e. the originally given file), so what we eventually get is
      cache=writeback on the temporary overlay and
      cache=writethrough,cache.no-flush=on on the real image file.
      
      This patch changes things so that the temporary overlay gets
      cache=unsafe again like it used to, and the real images get whatever the
      user specified. This means that cache.direct is now respected even with
      snapshot=on, and in the case of committing changes, the final flush is
      no longer ignored except explicitly requested by the user.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      73176bee
    • K
      blockdev: Snapshotting must not open second instance of old top · f86b8b58
      Kevin Wolf 提交于
      Calling bdrv_img_create() with a size of -1 means that it determines the
      size automatically by opening the backing file. However, in the case of
      live snapshots, the backing file is already opened and we must avoid
      opening the same image twice at the same time. Apart from that, just
      getting the size from the already existing BDS is a lot less overhead
      than opening a new instance.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NJeff Cody <jcody@redhat.com>
      f86b8b58
  6. 05 3月, 2016 1 次提交
  7. 22 2月, 2016 9 次提交
  8. 09 2月, 2016 1 次提交
    • E
      qapi: Swap visit_* arguments for consistent 'name' placement · 51e72bc1
      Eric Blake 提交于
      JSON uses "name":value, but many of our visitor interfaces were
      called with visit_type_FOO(v, &value, name, errp).  This can be
      a bit confusing to have to mentally swap the parameter order to
      match JSON order.  It's particularly bad for visit_start_struct(),
      where the 'name' parameter is smack in the middle of the
      otherwise-related group of 'obj, kind, size' parameters! It's
      time to do a global swap of the parameter ordering, so that the
      'name' parameter is always immediately after the Visitor argument.
      
      Additional reason in favor of the swap: the existing include/qjson.h
      prefers listing 'name' first in json_prop_*(), and I have plans to
      unify that file with the qapi visitors; listing 'name' first in
      qapi will minimize churn to the (admittedly few) qjson.h clients.
      
      Later patches will then fix docs, object.h, visitor-impl.h, and
      those clients to match.
      
      Done by first patching scripts/qapi*.py by hand to make generated
      files do what I want, then by running the following Coccinelle
      script to affect the rest of the code base:
       $ spatch --sp-file script `git grep -l '\bvisit_' -- '**/*.[ch]'`
      I then had to apply some touchups (Coccinelle insisted on TAB
      indentation in visitor.h, and botched the signature of
      visit_type_enum() by rewriting 'const char *const strings[]' to
      the syntactically invalid 'const char*const[] strings').  The
      movement of parameters is sufficient to provoke compiler errors
      if any callers were missed.
      
          // Part 1: Swap declaration order
          @@
          type TV, TErr, TObj, T1, T2;
          identifier OBJ, ARG1, ARG2;
          @@
           void visit_start_struct
          -(TV v, TObj OBJ, T1 ARG1, const char *name, T2 ARG2, TErr errp)
          +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
           { ... }
      
          @@
          type bool, TV, T1;
          identifier ARG1;
          @@
           bool visit_optional
          -(TV v, T1 ARG1, const char *name)
          +(TV v, const char *name, T1 ARG1)
           { ... }
      
          @@
          type TV, TErr, TObj, T1;
          identifier OBJ, ARG1;
          @@
           void visit_get_next_type
          -(TV v, TObj OBJ, T1 ARG1, const char *name, TErr errp)
          +(TV v, const char *name, TObj OBJ, T1 ARG1, TErr errp)
           { ... }
      
          @@
          type TV, TErr, TObj, T1, T2;
          identifier OBJ, ARG1, ARG2;
          @@
           void visit_type_enum
          -(TV v, TObj OBJ, T1 ARG1, T2 ARG2, const char *name, TErr errp)
          +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
           { ... }
      
          @@
          type TV, TErr, TObj;
          identifier OBJ;
          identifier VISIT_TYPE =~ "^visit_type_";
          @@
           void VISIT_TYPE
          -(TV v, TObj OBJ, const char *name, TErr errp)
          +(TV v, const char *name, TObj OBJ, TErr errp)
           { ... }
      
          // Part 2: swap caller order
          @@
          expression V, NAME, OBJ, ARG1, ARG2, ERR;
          identifier VISIT_TYPE =~ "^visit_type_";
          @@
          (
          -visit_start_struct(V, OBJ, ARG1, NAME, ARG2, ERR)
          +visit_start_struct(V, NAME, OBJ, ARG1, ARG2, ERR)
          |
          -visit_optional(V, ARG1, NAME)
          +visit_optional(V, NAME, ARG1)
          |
          -visit_get_next_type(V, OBJ, ARG1, NAME, ERR)
          +visit_get_next_type(V, NAME, OBJ, ARG1, ERR)
          |
          -visit_type_enum(V, OBJ, ARG1, ARG2, NAME, ERR)
          +visit_type_enum(V, NAME, OBJ, ARG1, ARG2, ERR)
          |
          -VISIT_TYPE(V, OBJ, NAME, ERR)
          +VISIT_TYPE(V, NAME, OBJ, ERR)
          )
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-Id: <1454075341-13658-19-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      51e72bc1
  9. 05 2月, 2016 1 次提交
    • P
      all: Clean up includes · d38ea87a
      Peter Maydell 提交于
      Clean up includes so that osdep.h is included first and headers
      which it implies are not included manually.
      
      This commit was created with scripts/clean-includes.
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      Message-id: 1454089805-5470-16-git-send-email-peter.maydell@linaro.org
      d38ea87a
  10. 03 2月, 2016 4 次提交
  11. 20 1月, 2016 1 次提交
  12. 13 1月, 2016 1 次提交
    • M
      error: Use error_reportf_err() where it makes obvious sense · c29b77f9
      Markus Armbruster 提交于
      Done with this Coccinelle semantic patch
      
          @@
          expression FMT, E, S;
          expression list ARGS;
          @@
          -    error_report(FMT, ARGS, error_get_pretty(E));
          +    error_reportf_err(E, FMT/*@@@*/, ARGS);
          (
          -    error_free(E);
          |
      	 exit(S);
          |
      	 abort();
          )
      
      followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
      because I can't figure out how to make Coccinelle transform strings.
      
      We now use the error whole instead of just its message obtained with
      error_get_pretty().  This avoids suppressing its hint (see commit
      50b7b000), but I can't see how the errors touched in this commit could
      come with hints.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <1450452927-8346-12-git-send-email-armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      c29b77f9
  13. 08 1月, 2016 1 次提交