)]}'
{
  "commit": "7e3d98dd31b6de53923bfb0f04b23b42a94f3829",
  "tree": "823ea4e705913bc101d0b4c95572e1ac35d3df22",
  "parents": [
    "8bfea15ddac3a0ae832f181653c36e020f24f007"
  ],
  "author": {
    "name": "Kevin Wolf",
    "email": "kwolf@redhat.com",
    "time": "Fri Apr 25 12:36:07 2014 +0200"
  },
  "committer": {
    "name": "Kevin Wolf",
    "email": "kwolf@redhat.com",
    "time": "Wed Apr 30 11:05:00 2014 +0200"
  },
  "message": "Revert \"block: another bdrv_append fix\"\n\nThis reverts commit 3a389e7926750cba5c83f662b1941888b2bebc04. The commit\nwas wrong and what it tried to fix just works today without any change.\n\nWhat the commit tried to fix:\n\n    When creating live snapshots, the new image file is opened with\n    BDRV_O_NO_BACKING because the whole backing chain is already opened.\n    It is then appended to the chain using bdrv_append(). The result of\n    this was that the image had a backing file, but BDRV_O_NO_BACKING\n    was still set. This is obviously inconsistent.\n\n    There used to be some places in qemu that closed and image and then\n    opened it again, with its old flags (a bdrv_open()/close() sequence\n    involves reopening the whole backing file chain, too). In this case\n    the BDRV_O_NO_BACKING flag meant that the backing chain wasn\u0027t\n    reopened and only the top layer was left.\n\n    (Most, but not all of these places are replaced by bdrv_reopen()\n    today, which doesn\u0027t touch the backing files at all.)\n\n    Other places that looked at bs-\u003eopen_flags weren\u0027t interested in\n    BDRV_O_NO_BACKING, so no breakage there.\n\nWhat it actually did:\n\n    The commit moved the BDRV_O_NO_BACKING away to the backing file.\n    Because the bdrv_open()/close() sequences only looked at the flags\n    of the top level BlockDriverState and used it for the whole chain,\n    the flag didn\u0027t hurt there any more. Obviously, it is still\n    inconsistent because the backing file may have another backing file,\n    but without practical impact.\n\n    At the same time, it swapped all other flags. This is practically\n    irrelevant as long as live snapshots only allow opening the new\n    layer with the same flags as the old top layer. It still doesn\u0027t\n    make any sense, and it is a time bomb that explodes as soon as the\n    flags can differ.\n\n    bdrv_append_temp_snapshot() is such a case: It adds the new flag\n    BDRV_O_TEMPORARY for the temporary snapshot. The swapping of commit\n    3a389e79 results in the following nonsensical configuration:\n\n    bs-\u003eopen_flags:                     BDRV_O_TEMPORARY cleared\n    bs-\u003efile-\u003eopen_flags:               BDRV_O_TEMPORARY set\n    bs-\u003ebacking_hd-\u003eopen_flags:         BDRV_O_TEMPORARY set\n    bs-\u003ebacking_hd-\u003efile-\u003eopen_flags:   BDRV_O_TEMPORARY cleared\n\n    We\u0027re still lucky because the format layer ignores the flag and the\n    protocol layer happens to get the right value, but sooner or later\n    this is bound to go wrong...\n\nWhat the right fix would have been:\n\n    Simply clear the BDRV_O_NO_BACKING flag when the BlockDriverState is\n    appended to an existing backing file chain, because now it does have\n    a backing file.\n\n    Commit 4ddc07ca already implemented this silently in bdrv_append(),\n    so we don\u0027t have to come up with a new fix.\n\nSigned-off-by: Kevin Wolf \u003ckwolf@redhat.com\u003e\nReviewed-by: Max Reitz \u003cmreitz@redhat.com\u003e\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "c094075e4bddaf09a44848fce6ab9a5fc9b4f2d2",
      "old_mode": 33188,
      "old_path": "block.c",
      "new_id": "9f6f07e75ccf8a554bedc75e554ab25c21501993",
      "new_mode": 33188,
      "new_path": "block.c"
    }
  ]
}
