1. 25 2月, 2019 16 次提交
    • K
      nbd: Move nbd_read_eof() to nbd/client.c · a7b78fc9
      Kevin Wolf 提交于
      The only caller of nbd_read_eof() is nbd_receive_reply(), so it doesn't
      have to live in the header file, but can move next to its caller.
      
      Also add the missing coroutine_fn to the function and its caller.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      a7b78fc9
    • K
      io: Remove redundant read/write_coroutine assignments · 2a239e6e
      Kevin Wolf 提交于
      qio_channel_yield() now updates ioc->read_write/coroutine and calls
      qio_channel_set_aio_fd_handlers(), so the code in the handlers has
      become redundant and can be removed.
      
      This does not make a difference in intermediate states because
      aio_co_wake() really enters the coroutine immediately here: These
      handlers are never run in coroutine context, and we're in the right
      AioContext because qio_channel_attach_aio_context() asserts that the
      handlers are inactive.
      
      To make these conditions more obvious, assert the right AioContext.
      Suggested-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      2a239e6e
    • K
      io: Make qio_channel_yield() interruptible · 6886ceaf
      Kevin Wolf 提交于
      Similar to how qemu_co_sleep_ns() allows preemption from an external
      coroutine entry, allow reentering qio_channel_yield() early.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      6886ceaf
    • K
      nbd: Restrict connection_co reentrance · 5ad81b49
      Kevin Wolf 提交于
      nbd_client_attach_aio_context() schedules connection_co in the new
      AioContext and this way reenters it in any arbitrary place that has
      yielded. We can restrict this a bit to the function call where the
      coroutine actually sits waiting when it's idle.
      
      This doesn't solve any bug yet, but it shows where in the code we need
      to support this random reentrance and where we don't have to care.
      
      Add FIXME comments for the existing bugs that the rest of this series
      will fix.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      5ad81b49
    • K
      virtio-blk: Increase in_flight for request restart BH · 680f2002
      Kevin Wolf 提交于
      virtio_blk_dma_restart_bh() submits new requests, so in order to make
      sure that these requests are not started inside a drained section of the
      attached BlockBackend, we need to make sure that draining the
      BlockBackend waits for the BH to be executed.
      
      This BH is still questionable because its scheduled in the main thread
      instead of the configured iothread. Leave a FIXME comment for this.
      
      But with this fix, enabling the data plane at least waits for these
      requests (in bdrv_set_aio_context()) instead of changing the AioContext
      under their feet and making them run in the wrong thread, causing
      crashes and failures (e.g. due to missing locking).
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      680f2002
    • K
      block-backend: Make blk_inc/dec_in_flight public · c90e2a9c
      Kevin Wolf 提交于
      For some users of BlockBackends, just increasing the in_flight counter
      is easier than implementing separate handlers in BlockDevOps. Make the
      helper functions for this public.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      c90e2a9c
    • D
      qemu-img: fix error reporting for -object · 334c43e2
      Daniel P. Berrangé 提交于
      Error reporting for user_creatable_add_opts_foreach was changed so that
      it no longer called 'error_report_err' in:
      
        commit 7e1e0c11
        Author: Markus Armbruster <armbru@redhat.com>
        Date:   Wed Oct 17 10:26:43 2018 +0200
      
          qom: Clean up error reporting in user_creatable_add_opts_foreach()
      
      Some callers were updated to pass in "&error_fatal" but all the ones in
      qemu-img were left passing NULL. As a result all errors went to
      /dev/null instead of being reported to the user.
      Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NStefano Garzarella <sgarzare@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      334c43e2
    • A
      commit: Replace commit_top_bs on failure after deleting the block job · 2468eed3
      Alberto Garcia 提交于
      If there's an error in commit_start() then the block job must be
      deleted before replacing commit_top_bs, otherwise it will fail because
      of lack of permissions. This happens since the permission system was
      introduced in 8dfba279.
      
      Fortunately this bug doesn't seem to be possible to reproduce at the
      moment without changing the code.
      Signed-off-by: NAlberto Garcia <berto@igalia.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      2468eed3
    • D
      block: don't set the same context · 57830a49
      Denis Plotnikov 提交于
      Adds a fast path on aio context setting preventing
      unnecessary context setting routine.
      Also, it prevents issues with cyclic walk of child
      bds-es appeared because of registering aio walking
      notifiers:
      
      Call stack:
      
      0  __GI_raise
      1  __GI_abort
      2  __assert_fail_base
      3  __GI___assert_fail
      4  bdrv_detach_aio_context (bs=0x55f54d65c000)      <<<
      5  bdrv_detach_aio_context (bs=0x55f54fc8a800)
      6  bdrv_set_aio_context (bs=0x55f54fc8a800, ...)
      7  block_job_attached_aio_context
      8  bdrv_attach_aio_context (bs=0x55f54d65c000, ...) <<<
      9  bdrv_set_aio_context (bs=0x55f54d65c000)
      10 blk_set_aio_context
      11 virtio_blk_data_plane_stop
      12 virtio_bus_stop_ioeventfd
      13 virtio_vmstate_change
      14 vm_state_notify (running=0, state=RUN_STATE_SHUTDOWN)
      15 do_vm_stop (state=RUN_STATE_SHUTDOWN, send_stop=true)
      16 vm_stop (state=RUN_STATE_SHUTDOWN)
      17 main_loop_should_exit
      18 main_loop
      19 main
      
      This can happen because of "new" context attachment to VM disk bds.
      When attaching a new context the corresponding aio context handler is
      called for each of aio_notifiers registered on the VM disk bds context.
      Among those handlers, there is the block_job_attached_aio_context handler
      which sets a new aio context for the block job bds. When doing so,
      the old context is detached from all the block job bds children and one of
      them is the VM disk bds, serving as backing store for the blockjob bds,
      although the VM disk bds is actually the initializer of that process.
      Since the VM disk bds is protected with walking_aio_notifiers flag
      from double processing in recursive calls, the assert fires.
      Signed-off-by: NDenis Plotnikov <dplotnikov@virtuozzo.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      57830a49
    • D
      qcow2-snapshot: remove redundant find_snapshot_by_id_and_name call · 161e612d
      Daniel Henrique Barboza 提交于
      In qcow2_snapshot_create there is the following code block:
      
          /* Generate an ID */
          find_new_snapshot_id(bs, sn_info->id_str, sizeof(sn_info->id_str));
      
          /* Check that the ID is unique */
          if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
              return -EEXIST;
          }
      
      find_new_snapshot_id cycles through all snapshots, getting the id_str
      as an unsigned long int, calculating the max id_max value of all the
      existing id_strs and writing in the id_str pointer id_max + 1:
      
          for(i = 0; i < s->nb_snapshots; i++) {
              sn = s->snapshots + i;
              id = strtoul(sn->id_str, NULL, 10);
              if (id > id_max)
                  id_max = id;
          }
          snprintf(id_str, id_str_size, "%lu", id_max + 1);
      
      Here, sn_info->id_str will have the unique value id_max + 1. Right
      after that, find_snapshot_by_id_and_name is called with
      id = sn_info->id_str and name = NULL. This will cause the function
      to execute the following:
      
          } else if (id) {
              for (i = 0; i < s->nb_snapshots; i++) {
                  if (!strcmp(s->snapshots[i].id_str, id)) {
                      return i;
                  }
              }
          }
      
      In short, we're searching the existing snapshots to see if sn_info->id_str
      matches any existing id, right after we set in the previous line a
      sn_info->id_str value that is already unique.
      
      The first code block goes way back to commit 585f8587, a 2006 commit from
      Fabrice Bellard that simply says "new qcow2 disk image format". No more
      info is provided about this logic in any subsequent commits that moved
      this code block around.
      
      I can't say about the original design, but the current logic is redundant.
      bdrv_snapshot_create is called in aio_context lock, forbidding any
      concurrent call to accidentally create a new snapshot between
      the find_new_snapshot_id and find_snapshot_by_id_and_name calls. What
      we're ending up doing is to cycle through the snapshots two times
      for no viable reason.
      
      This patch eliminates the redundancy by removing the 'id is unique'
      check that calls find_snapshot_by_id_and_name.
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      161e612d
    • D
      block/snapshot: remove bdrv_snapshot_delete_by_id_or_name · 8c04093c
      Daniel Henrique Barboza 提交于
      After the previous patch, the only instance of this function left
      is inside qemu-img.c.
      
      qemu-img is using it inside the 'img_snapshot' function to delete
      snapshots in the SNAPSHOT_DELETE case, based on a "snapshot_name"
      string that refers to the tag, not ID, of the QEMUSnapshotInfo struct.
      This can be verified by checking the SNAPSHOT_CREATE case that
      comes shortly before SNAPSHOT_DELETE. In that case, the same
      "snapshot_name" variable is being strcpy to the 'name' field
      of the QEMUSnapshotInfo struct sn:
      
      pstrcpy(sn.name, sizeof(sn.name), snapshot_name);
      
      Based on that, it is unlikely that "snapshot_name" might contain
      an "id" in SNAPSHOT_DELETE.
      
      This patch changes SNAPSHOT_DELETE to use snapshot_find() and
      snapshot_delete() instead of bdrv_snapshot_delete_by_id_or_name.
      After that, there is no instances left of bdrv_snapshot_delete_by_id_or_name
      in the code, so it is safe to remove it entirely.
      Suggested-by: NMurilo Opsfelder Araujo <muriloo@linux.ibm.com>
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      8c04093c
    • D
      block/snapshot.c: eliminate use of ID input in snapshot operations · 6ca08045
      Daniel Henrique Barboza 提交于
      At this moment, QEMU attempts to create/load/delete snapshots
      by using either an ID (id_str) or a name. The problem is that the code
      isn't consistent of whether the entered argument is an ID or a name,
      causing unexpected behaviors.
      
      For example, when creating snapshots via savevm <arg>, what happens is that
      "arg" is treated as both name and id_str. In a guest without snapshots, create
      a single snapshot via savevm:
      
      (qemu) savevm 0
      (qemu) info snapshots
      List of snapshots present on all disks:
      ID        TAG                 VM SIZE                DATE       VM CLOCK
      --        0                      741M 2018-07-31 13:39:56   00:41:25.313
      
      A snapshot with name "0" is created. ID is hidden from the user, but the
      ID is a non-zero integer that starts at "1". Thus, this snapshot has
      id_str=1, TAG="0". Creating a second snapshot with arg = 1, the first one
      is deleted:
      
      (qemu) savevm 1
      (qemu) info snapshots
      List of snapshots present on all disks:
      ID        TAG                 VM SIZE                DATE       VM CLOCK
      --        1                      741M 2018-07-31 13:42:14   00:41:55.252
      
      What happened?
      
      - when creating the second snapshot, a verification is done inside
      bdrv_all_delete_snapshot to delete any existing snapshots that matches an
      string argument. Here, the code calls bdrv_all_delete_snapshot("1", ...);
      
      - bdrv_all_delete_snapshot calls bdrv_snapshot_find(..., "1") for each
      BlockDriverState of the guest. And this is where things goes tilting:
      bdrv_snapshot_find does a search by both id_str and name. It finds
      out that there is a snapshot that has id_str = 1, stores a reference
      to the snapshot in the sn_info pointer and then returns match found;
      
      - since a match was found, a call to bdrv_snapshot_delete_by_id_or_name() is
      made. This function ignores the pointer written by bdrv_snapshot_find. Instead,
      it deletes the snapshot using bdrv_snapshot_delete() calling it first with
      id_str = 1. If it fails to delete, then it calls it again with name = 1.
      
      - after all that, QEMU creates the new snapshot, that has id_str = 1 and
      name = 1. The user is left wondering that happened with the first snapshot
      created. Similar bugs can be triggered when using loadvm and delvm.
      
      Before contemplating discarding the use of ID input in these operations,
      I've searched the code of what would be the implications. My findings
      are:
      
      - the RBD and Sheepdog drivers don't care. Both uses the 'name' field as
      key in their logic, making id_str = name when appropriate.
      replay-snapshot.c does not make any special use of id_str;
      
      - qcow2 uses id_str as an unique identifier but it is automatically
      calculated, not being influenced by user input. Other than that, there are
      no distinguish operations made only with id_str;
      
      - in blockdev.c, the delete operation uses a match of both id_str AND
      name. Given that id_str is either a copy of 'name' or auto-generated,
      we're fine here.
      
      This gives motivation to not consider ID as a valid user input in HMP
      commands - sticking with 'name' input only is more consistent. To
      accomplish that, the following changes were made in this patch:
      
      - bdrv_snapshot_find() does not match for id_str anymore, only 'name'. The
      function is called in save_snapshot(), load_snapshot(), bdrv_all_delete_snapshot()
      and bdrv_all_find_snapshot(). This change makes the search function more
      predictable and does not change the behavior of any underlying code that uses
      these affected functions, which are related to HMP (which is fine) and the
      main loop inside vl.c (which doesn't care about it anyways);
      
      - bdrv_all_delete_snapshot() does not call bdrv_snapshot_delete_by_id_or_name
      anymore. Instead, it uses the pointer returned by bdrv_snapshot_find to
      erase the snapshot with the exact match of id_str an name. This function
      is called in save_snapshot and hmp_delvm, thus this change  produces the
      intended effect;
      
      - documentation changes to reflect the new behavior. I consider this to
      be an API fix instead of an API change - the user was already creating
      snapshots using 'name', but now he/she will also enjoy a consistent
      behavior.
      
      Ideally we would get rid of the id_str field entirely, but this would have
      repercussions on existing snapshots. Another day perhaps.
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Acked-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      6ca08045
    • J
      MAINTAINERS: Remove myself as block maintainer · 5f5246b6
      Jeff Cody 提交于
      I'll not be involved in day-to-day qemu development.  Remove myself as
      maintainer from the remainder of the network block drivers, and revert
      them to the general block layer maintainership.
      
      Move 'sheepdog' to the 'Odd Fixes' support level.
      
      For VHDX, added my personal email address as a maintainer, as I can
      answer questions or send the occassional bug fix.  Leaving it as
      'Supported', instead of 'Odd Fixes', because I think the rest of the
      block layer maintainers and developers will upkeep it as well, if
      needed.
      Signed-off-by: NJeff Cody <jcody@redhat.com>
      Acked-by: NMax Reitz <mreitz@redhat.com>
      Message-Id: <63e205cb84c8f0a10c1bc6d5d6856d72ceb56e41.1537984851.git.jcody@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      5f5246b6
    • J
      MAINTAINERS: Replace myself with John Snow for block jobs · 03283d64
      Jeff Cody 提交于
      I'll not be involved with day-to-day qemu development, and John
      Snow is a block jobs wizard.  Have him take over block job
      maintainership duties.
      Signed-off-by: NJeff Cody <jcody@redhat.com>
      Acked-by: NJohn Snow <jsnow@redhat.com>
      Message-Id: <d56d7c6592e7d68aa72764e9616878394bffbc14.1537984851.git.jcody@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      03283d64
    • P
      Merge remote-tracking branch 'remotes/kraxel/tags/vga-20190222-pull-request' into staging · 59a568b5
      Peter Maydell 提交于
      vga: bugfixes and edid support for virtio-vga
      
      # gpg: Signature made Fri 22 Feb 2019 08:24:25 GMT
      # gpg:                using RSA key 4CB6D8EED3E87138
      # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full]
      # gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>" [full]
      # gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full]
      # Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138
      
      * remotes/kraxel/tags/vga-20190222-pull-request:
        display/virtio: add edid support.
        virtio-gpu: remove useless 'waiting' field
        virtio-gpu: block both 2d and 3d rendering
        virtio-gpu: remove unused config_size
        virtio-gpu: remove unused qdev
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      59a568b5
    • P
      Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190222-pull-request' into staging · 8a4c08b1
      Peter Maydell 提交于
      ui: add support for -display spice-app
      ui: gtk+sdl bugfixes.
      
      # gpg: Signature made Fri 22 Feb 2019 07:53:13 GMT
      # gpg:                using RSA key 4CB6D8EED3E87138
      # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full]
      # gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>" [full]
      # gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full]
      # Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138
      
      * remotes/kraxel/tags/ui-20190222-pull-request:
        display: add -display spice-app launching a Spice client
        spice: use a default name for the server
        qapi: document DisplayType enum
        build-sys: add gio-2.0 check
        char: register spice ports after spice started
        char: move SpiceChardev and open_spice_port() to spice.h header
        spice: do not stop spice if VM is paused
        spice: merge options lists
        spice: avoid spice runtime assert
        char/spice: discard write() if backend is disconnected
        char/spice: trigger HUP event
        ui/gtk: Fix the license information
        sdl2: drop qemu_input_event_send_key_qcode call
        spice: set device address and device display ID in QXL interface
        kbd-state: don't block auto-repeat events
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      8a4c08b1
  2. 22 2月, 2019 24 次提交