Re: [Qemu-devel] [RESEND] [PATCH 0/6] virtio: refactor host notifiers

[Fam Zheng]

On Fri, 06/10 11:04, Cornelia Huck wrote:
> Hi,
> 
> this is a resend of the host notifier refactoring I last sent out
> on 2016/04/25. Just rebased on current master and pushed out to
> github.com/cohuck/qemu host-notifier-refactor.
> 
> Blurb from the last posting follows:
> 
> This still fixes an issue where we have no ioeventfd for a short
> time, but it does not break our necks any more with the recent
> fixes. Still, I think this is a net win, especially considering
> we remove a lot of similar code from the individual transports.

Agreed. And the changes look good to me, thanks.

Reviewed-by: Fam Zheng 

> 
> Cornelia Huck (6):
>   virtio-bus: common ioeventfd infrastructure
>   virtio-bus: have callers tolerate new host notifier api
>   virtio-ccw: convert to ioeventfd callbacks
>   virtio-pci: convert to ioeventfd callbacks
>   virtio-mmio: convert to ioeventfd callbacks
>   virtio-bus: remove old set_host_notifier callback
> 
>  hw/block/dataplane/virtio-blk.c |   6 +-
>  hw/s390x/virtio-ccw.c   | 133 
> ++--
>  hw/scsi/virtio-scsi-dataplane.c |   9 ++-
>  hw/virtio/vhost.c   |  13 ++--
>  hw/virtio/virtio-bus.c  | 132 +++
>  hw/virtio/virtio-mmio.c | 128 +-
>  hw/virtio/virtio-pci.c  | 124 +
>  include/hw/virtio/virtio-bus.h  |  31 +-
>  8 files changed, 303 insertions(+), 273 deletions(-)
> 
> -- 
> 2.6.6
> 

Re: [Qemu-devel] [PATCH V3] block/iscsi: allow caching of the allocation map

[Fam Zheng]

On Tue, 05/24 10:40, Peter Lieven wrote:
> until now the allocation map was used only as a hint if a cluster
> is allocated or not. If a block was not allocated (or Qemu had
> no info about the allocation status) a get_block_status call was
> issued to check the allocation status and possibly avoid
> a subsequent read of unallocated sectors. If a block known to be
> allocated the get_block_status call was omitted. In the other case
> a get_block_status call was issued before every read to avoid
> the necessity for a consistent allocation map. To avoid the
> potential overhead of calling get_block_status for each and
> every read request this took only place for the bigger requests.
> 
> This patch enhances this mechanism to cache the allocation
> status and avoid calling get_block_status for blocks where
> the allocation status has been queried before. This allows
> for bypassing the read request even for smaller requests and
> additionally omits calling get_block_status for known to be
> unallocated blocks.
> 
> Signed-off-by: Peter Lieven 
> ---
> v2->v3: - fix wording errors [Fam]
> - reinit allocmap only if allocmap is present in
>   bdrv_reopen_commit

I did an incremental review and the diff-of-diff looks good to me, so:

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v2 2/3] hw/iommu: enable iommu with -device

[Peter Xu]

On Thu, Jun 02, 2016 at 11:15:54PM +0300, Marcel Apfelbaum wrote:

[...]

>  static void vtd_realize(DeviceState *dev, Error **errp)
>  {
> +PCIBus *bus = PC_MACHINE(qdev_get_machine())->bus;
>  IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
>  
>  VTD_DPRINTF(GENERAL, "");
> @@ -2029,6 +2043,9 @@ static void vtd_realize(DeviceState *dev, Error **errp)
>  s->vtd_as_by_busptr = g_hash_table_new_full(vtd_uint64_hash, 
> vtd_uint64_equal,
>g_free, g_free);
>  vtd_init(s);
> +sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
> +bus->iommu_fn = vtd_host_dma_iommu;
> +bus->iommu_opaque = dev;

Here, shall we still use pci_setup_iommu() to keep the two fields
private for pci framework?

Btw, I am rebasing Intel IR work onto this patchset, but encountered
issues (guest hang, or errornous interrupts) when guest specify more
than 1 vcpus (everything is cool as long as vcpu=1). Maybe there is
something wrong during the rebase, still investigating. Please shoot
if there is any clue.

Thanks,

-- peterx

Re: [Qemu-devel] [PATCH v3 0/5] block/mirror: Fix (?) target backing BDS

[Fam Zheng]

On Fri, 06/10 20:57, Max Reitz wrote:
> Issue #1: If the target image does not have a backing BDS before mirror
> completion, qemu tries really hard to give it a backing BDS. If the
> source has a backing BDS, it will actually always "succeed".
> In some cases, the target is not supposed to have a backing BDS, though
> (absolute-paths: because of sync=full; existing: because the target
> image does not have a backing file; blockdev-mirror: because of an
> explicit "backing": ""). Then, this is pretty bad behavior.
> 
> This should generally not change the target's visible data, but it still
> is ugly.
> 
> Issue #2: Currently the backing chain of the target is basically opened
> using bdrv_open_backing_file() (except for sometimes™). This results in
> multiple BDSs for a single physical file, which is bad. In most use
> cases, this is only temporary, but it still is bad.
> 
> If we can reuse the existing backing chain of the source (which is with
> drive-mirror in "absolute-paths" mode), we should just do so.

Looks good overall. I left a few trivial comments/questions in individual
patches.

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v3 4/5] iotests: Add test for post-mirror backing chains

[Fam Zheng]

On Fri, 06/10 20:57, Max Reitz wrote:
> +import os
> +import stat
> +import time

Unused import 'stat' and 'time'?

> +import iotests
> +from iotests import qemu_img

Re: [Qemu-devel] [PATCH v3 3/5] block/null: Implement bdrv_refresh_filename()

[Fam Zheng]

On Fri, 06/10 20:57, Max Reitz wrote:
> Signed-off-by: Max Reitz 

The commit message could go a little more informative. Seems nothing special in
the added callback, aren't things supposed to just work without this patch?
What is missing?

> ---
>  block/null.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/block/null.c b/block/null.c
> index 396500b..b511010 100644
> --- a/block/null.c
> +++ b/block/null.c
> @@ -12,6 +12,8 @@
>  
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qstring.h"
>  #include "block/block_int.h"
>  
>  #define NULL_OPT_LATENCY "latency-ns"
> @@ -223,6 +225,20 @@ static int64_t coroutine_fn 
> null_co_get_block_status(BlockDriverState *bs,
>  }
>  }
>  
> +static void null_refresh_filename(BlockDriverState *bs, QDict *opts)
> +{
> +QINCREF(opts);
> +qdict_del(opts, "filename");

Why is this qdict_del necessary?

> +
> +if (!qdict_size(opts)) {
> +snprintf(bs->exact_filename, sizeof(bs->exact_filename), "%s://",
> + bs->drv->format_name);
> +}
> +
> +qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
> +bs->full_open_options = opts;
> +}
> +
>  static BlockDriver bdrv_null_co = {
>  .format_name= "null-co",
>  .protocol_name  = "null-co",
> @@ -238,6 +254,8 @@ static BlockDriver bdrv_null_co = {
>  .bdrv_reopen_prepare= null_reopen_prepare,
>  
>  .bdrv_co_get_block_status   = null_co_get_block_status,
> +
> +.bdrv_refresh_filename  = null_refresh_filename,
>  };
>  
>  static BlockDriver bdrv_null_aio = {
> @@ -255,6 +273,8 @@ static BlockDriver bdrv_null_aio = {
>  .bdrv_reopen_prepare= null_reopen_prepare,
>  
>  .bdrv_co_get_block_status   = null_co_get_block_status,
> +
> +.bdrv_refresh_filename  = null_refresh_filename,
>  };
>  
>  static void bdrv_null_init(void)
> -- 
> 2.8.3
> 

[Qemu-devel] ext4 error when testing virtio-scsi & vhost-scsi

[Zhangfei Gao]

Here is one question about testing virtio-scsi & vhost-scsi.
I met ext4 error using fileio or iblock.
And after the error, the filesystem can not be remount next time in
guest os except mkfs.ext4 again.

Any suggestions?
Thanks in advance.


Basic steps.
fileio:
mount /dev/sda3 /mnt
dd if=/dev/zero of=test bs=1M count=1024


#targetcli

(targetcli) /> cd backstores/fileio

(targetcli) /> create name=file_backend file_or_dev=/mnt/test size=1G

(targetcli) /> cd /vhost

(targetcli) /> create wwn=naa.60014052cc816bf4

(targetcli) /> cd naa.60014052cc816bf4/tpgt1/luns

(targetcli) /> create /backstores/fileio/file_backend

(targetcli) /> cd /

(targetcli) /> saveconfig

(targetcli) /> exit

qemu.git/aarch64-softmmu/qemu-system-aarch64 \

   -enable-kvm -nographic -kernel Image \

   -device vhost-scsi-pci,wwpn=naa.60014052cc816bf4 \

   -m 512 -M virt -cpu host \

   -append "earlyprintk console=ttyAMA0 mem=512M rw"


After guest kernel is boot,

Mkfs.ext4 /dev/sda

Mount /dev/sda /mnt


sync; date; dd if=/dev/zero of=test bs=1M count=100; sync; date;


Ext4 error:

And can not be mounted next time.

[  762.387457] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  762.395622] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  762.403915] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  762.412263] EXT4-fs error (device sda) in ext4_ext_truncate:4661:
Corrupt filesystem

[  762.420613] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  762.428913] EXT4-fs error (device sda) in ext4_orphan_del:2896:
Corrupt filesystem

[  762.437262] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  762.445614] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  762.454516] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  762.462283] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[  767.370571] jbd2_journal_bmap: journal block not found at offset 13 on sda-8

[  767.371458] Aborting journal on device sda-8.

[  767.395583] EXT4-fs error: 564 callbacks suppressed

[  767.396173] EXT4-fs error (device sda) in ext4_da_write_end:2841: IO failure

[  767.412221] EXT4-fs error (device sda):
ext4_journal_check_start:56: Detected aborted journal

[  767.413325] EXT4-fs (sda): Remounting filesystem read-only

dd: writing '/mnt/test.bin': Read-only file system


blockio:

# targetcli

/> cd backstores/iblock

/backstores/iblock> create name=block_backend dev=/dev/sda4

/backstores/iblock> cd /vhost

/vhost> create

/vhost> ls

o- vhost  [1 Target]

 o- naa.60014053c5cc00ac .. [1 TPG]

   o- tpg1 . [naa.6001405830beacfa]

 o- luns . [0 LUNs]

/vhost> cd naa.60014053c5cc00ac/tpg1/luns

/vhost/naa.60...0ac/tpg1/luns> create /backstores/iblock/block_backend

/vhost/naa.60...0ac/tpg1/luns> cd /

/> saveconfig

qemu.git/aarch64-softmmu/qemu-system-aarch64 \

   -enable-kvm -nographic -kernel Image \

   -device vhost-scsi-pci,wwpn=naa.60014053c5cc00ac \

   -m 512 -M virt -cpu host \

   -append "earlyprintk console=ttyAMA0 mem=512M"


Mount /dev/sda /mnt

sync; date; dd if=/dev/zero of=/mnt/test bs=1M count=100; sync; date;


sync; date; sync; date; dd if=/dev/zero of=/mnt/test bs=1M count=100;

Thu Jan  1 00:01:16 UTC 1970

[   77.044879] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[   77.067334] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[   77.075623] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[   77.083970] EXT4-fs error (device sda) in ext4_ext_truncate:4661:
Corrupt filesystem

[   77.092322] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[   77.100619] EXT4-fs error (device sda) in ext4_orphan_del:2896:
Corrupt filesystem

[   77.108971] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[   77.117321] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[   77.126204] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem

[   77.133989] EXT4-fs error (device sda) in
ext4_reserve_inode_write:5172: Corrupt filesystem


[   82.025630] jbd2_journal_bmap: journal block not found at offset 10 on sda-8

[   82.026522] Aborting journal on device sda-8.

[   82.050642] EXT4-fs error: 563 callbacks suppressed

[   82.051278] EXT4-fs error (device sda) in ext4_da_write_end:2841: IO failure

[   82.067283] EXT4-fs error (device sda):
ext4_journal_check_start:56: Detected aborted journal

[   82.068372] EXT4-fs (sda): Remounting filesystem read-only

Re: [Qemu-devel] [PATCH v3 1/5] block: Allow replacement of a BDS by its overlay

[Fam Zheng]

On Fri, 06/10 20:57, Max Reitz wrote:
> change_parent_backing_link() asserts that the BDS to be replaced is not
> used as a backing file. However, we may want to replace a BDS by its
> overlay in which case that very link should not be redirected.
> 
> For instance, when doing a sync=none drive-mirror operation, we may have
> the following BDS/BB forest before block job completion:
> 
>   target
> 
>   base <- source <- BlockBackend
> 
> During job completion, we want to establish the source BDS as the
> target's backing node:
> 
>   target
> |
> v
>   base <- source <- BlockBackend
> 
> This makes the target a valid replacement for the source:
> 
>   target <- BlockBackend
> |
> v
>   base <- source
> 
> Without this modification to change_parent_backing_link() we have to
> inject the target into the graph before the source is its backing node,
> thus temporarily creating a wrong graph:
> 
>   target <- BlockBackend
> 
>   base <- source
> 
> Signed-off-by: Max Reitz 
> ---
>  block.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index f54bc25..dc76c159 100644
> --- a/block.c
> +++ b/block.c
> @@ -2224,9 +2224,23 @@ void bdrv_close_all(void)
>  static void change_parent_backing_link(BlockDriverState *from,
> BlockDriverState *to)
>  {
> -BdrvChild *c, *next;
> +BdrvChild *c, *next, *to_c;
>  
>  QLIST_FOREACH_SAFE(c, >parents, next_parent, next) {
> +if (c->role == _backing) {
> +/* @from is generally not allowed to be a backing file, except 
> for
> + * when @to is the overlay. In that case, @from may not be 
> replaced
> + * by @to as @to's backing node. */
> +QLIST_FOREACH(to_c, >children, next) {
> +if (to_c == c) {
> +break;
> +}
> +}

Why not just test "c->bs == to" here, why is it necessary to iterate through
to->children list?

Fam

> +if (to_c) {
> +continue;
> +}
> +}
> +
>  assert(c->role != _backing);
>  bdrv_ref(to);
>  bdrv_replace_child(c, to);
> -- 
> 2.8.3
> 

Re: [Qemu-devel] [RFC PATCH 0/3] x86: Add support for guest DMA dirty page tracking

[Zhou Jie]

Hi, Alex

On 2016/6/9 23:39, Alexander Duyck wrote:

On Thu, Jun 9, 2016 at 3:14 AM, Zhou Jie  wrote:

TO Alex
TO Michael

   In your solution you add a emulate PCI bridge to act as
   a bridge between direct assigned devices and the host bridge.
   Do you mean put all direct assigned devices to
   one emulate PCI bridge?
   If yes, this maybe bring some problems.

   We are writing a patchset to support aer feature in qemu.
   When assigning a vfio device with AER enabled, we must check whether
   the device supports a host bus reset (ie. hot reset) as this may be
   used by the guest OS in order to recover the device from an AER
   error.
   QEMU must therefore have the ability to perform a physical
   host bus reset using the existing vfio APIs in response to a virtual
   bus reset in the VM.
   A physical bus reset affects all of the devices on the host bus.
   Therefore all physical devices affected by a bus reset must be
   configured on the same virtual bus in the VM.
   And no devices unaffected by the bus reset,
   be configured on the same virtual bus.

   http://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg02989.html

Sincerely,
Zhou Jie


That makes sense, but I don't think you have to worry much about this
at this point at least on my side as this was mostly just theory and I
haven't had a chance to put any of it into practice as of yet.

My idea has been evolving on this for a while.  One thought I had is
that we may want to have something like an emulated IOMMU and if
possible we would want to split it up over multiple domains just so we
can be certain that the virtual interfaces and the physical ones
existed in separate domains.  In regards to your concerns perhaps what
we could do is put each assigned device into its own domain to prevent
them from affecting each other.  To that end we could probably break
things up so that each device effectively lives in its own PCIe slot
in the emulated system.  Then when we start a migration of the guest
the assigned device domains would then have to be tracked for unmap
and sync calls when the direction is from the device.

I will keep your concerns in mind in the future when I get some time
to look at exploring this solution further.

- Alex


I am thinking about the practice of migration of passthrough device.

In your solution, you use a vendor specific configuration space to
negotiate with guest.
If you put each assigned device into its own domain,
how can qemu negotiate with guest?
Add the vendor specific configuration space to every pci bus which
is assigned a passthrough device?

Sincerely
Zhou Jie

Re: [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups

[Fam Zheng]

On Fri, 06/10 18:05, Kevin Wolf wrote:
> This series contains a few cleanups with respect to the vmstate I/O functions.
> Apart from making the interface more consistent (writes were already vectored,
> but not reads), this makes use of the new byte-based .bdrv_co_preadv/pwritev
> callbacks in qcow2 to get rid of a few hacks, including bs->zero_beyond_eof.

Looks sane to me,

Reviewed-by: Fam Zheng 

(Kevin, this is based on your "qcow2: Implement .bdrv_co_preadv/pwritev"
series, right?)

Fam

> 
> Kevin Wolf (6):
>   block: Introduce bdrv_preadv()
>   block: Make .bdrv_load_vmstate() vectored
>   block: Allow .bdrv_load/save_vmstate() to return 0/-errno
>   block: Make bdrv_load/save_vmstate coroutine_fns
>   qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
>   block: Remove bs->zero_beyond_eof
> 
>  block.c   |   2 -
>  block/io.c| 173 
> +++---
>  block/qcow2.c |  28 ++--
>  block/sheepdog.c  |  13 +++-
>  include/block/block.h |   2 +
>  include/block/block_int.h |  13 ++--
>  6 files changed, 143 insertions(+), 88 deletions(-)
> 
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly

[Fam Zheng]

On Fri, 06/10 16:39, Eric Blake wrote:
> > +++ b/block/qcow2.c
> > @@ -2903,36 +2903,20 @@ static int qcow2_save_vmstate(BlockDriverState *bs, 
> > QEMUIOVector *qiov,
> >int64_t pos)
> >  {
> >  BDRVQcow2State *s = bs->opaque;
> > -int64_t total_sectors = bs->total_sectors;
> > -bool zero_beyond_eof = bs->zero_beyond_eof;
> > -int ret;
> >  
> >  BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
> > -bs->zero_beyond_eof = false;
> > -ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
> > -bs->zero_beyond_eof = zero_beyond_eof;
> > -
> > -/* bdrv_co_do_writev will have increased the total_sectors value to 
> > include
> > - * the VM state - the VM state is however not an actual part of the 
> > block
> > - * device, therefore, we need to restore the old value. */
> > -bs->total_sectors = total_sectors;
> > -
> > -return ret;
> > +return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
> > +qiov->size, qiov, 0);
> >  }
> 
> bs->drv->bdrv_co_pwritev() is an optional interface; not all the drivers
> have it yet.  Should you be asserting that it exists, and/or returning
> an error if it does not?

bs->drv is always qcow2 driver here so this is correct.

> 
> >  
> >  static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
> >int64_t pos)
> >  {
> >  BDRVQcow2State *s = bs->opaque;
> > -bool zero_beyond_eof = bs->zero_beyond_eof;
> > -int ret;
> >  
> >  BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
> > -bs->zero_beyond_eof = false;
> > -ret = bdrv_preadv(bs, qcow2_vm_state_offset(s) + pos, qiov);
> > -bs->zero_beyond_eof = zero_beyond_eof;
> > -
> > -return ret;
> > +return bs->drv->bdrv_co_preadv(bs, qcow2_vm_state_offset(s) + pos,
> > +   qiov->size, qiov, 0);
> 
> Ditto.
> 

Fam

Re: [Qemu-devel] [PATCH v8 11/12] vfio: register aer resume notification handler for aer resume

[Zhou Jie]

Hi, Alex


It seems like we have a number of questions open in the thread with MST
from the previous version, particularly whether we should actually drop
the resume notifier and block the reset in the kernel.  The concern
being that it's not very well specified what we can and cannot do
between the error interrupt and the resume interrupt.  We'd probably
need some other indicate of whether the host has this capability,
perhaps a flag in vfio_device_info.  Appreciate your opinions there.
Thanks,

Alex


Have you had a conclusion with MST?

How about the process like following?
1. Detect support for VFIO reset blocking.
   Maybe use vfio_device_info.
2. Immediately notify the VM on error detected.
3. Invoke ioctl(VFIO_DEVICE_PCI_HOT_RESET).
   If kernel is doing reset, then block in ioctl.

   Can I ignore the resume notifier?
   else
   Shall ioctl block until after receiving resume notification?

Sincerely
Zhou Jie

Re: [Qemu-devel] [PATCH] block-backend: allow flush on devices with open tray

[Fam Zheng]

On Fri, 06/10 17:59, John Snow wrote:
> If a device still has an attached BDS because the medium has not yet
> been removed, we will be unable to migrate to a new host because
> blk_flush will return an error for that backend.
> 
> Replace the call to blk_is_available to blk_is_inserted to weaken
> the check and allow flushes from the backend to work, while still
> disallowing flushes from the frontend/device model to work.
> 
> This fixes a regression present in 2.6.0 caused by the following commit:
> fe1a9cbc339bb54d20f1ca4c1e8788d16944d5cf
> block: Move some bdrv_*_all() functions to BB
> 
> Signed-off-by: John Snow 
> ---
>  block/block-backend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 34500e6..d1e875e 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1122,7 +1122,7 @@ int blk_co_flush(BlockBackend *blk)
>  
>  int blk_flush(BlockBackend *blk)
>  {
> -if (!blk_is_available(blk)) {
> +if (!blk_is_inserted(blk)) {
>  return -ENOMEDIUM;
>  }
>  
> -- 
> 2.4.11
> 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH] block: Avoid bogus flags during mirroring

[Fam Zheng]

On Sat, 06/11 19:18, Eric Blake wrote:
> Commit e253f4b8 converted mirroring from sector-based bdrv_aio_*
> to byte-based blk_aio_*, but failed to account for the subtle
> difference in signatures (the former takes a semi-redundant length,
> the latter takes a flags parameter).  Since all of our flags are
> currently smaller in size than BDRV_SECTOR_SIZE, it has no ill
> effects until we either perform sub-sector mirroring, or we start
> asserting that no unexpected flags are set.  I found it while
> testing new asserts when qemu-iotests 132 started warning about an
> unknown flag 0x20.
> 
> Add an assert to help us catch any other improper flags.
> 
> Signed-off-by: Eric Blake 
> ---
>  block/block-backend.c | 2 ++
>  block/mirror.c| 6 ++
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 34500e6..a5dc6e3 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -945,6 +945,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, 
> int64_t offset, int bytes,
>  acb->bh = NULL;
>  acb->has_returned = false;
> 
> +assert(flags <= 0x1f);
> +

Maybe define BDRV_REQ_FLAGS_MAX in include/block/block.h so it's easier to keep
this assertion valid when we introduce more flags in the future?

Otherwise the patch looks good.

Fam

>  co = qemu_coroutine_create(co_entry);
>  qemu_coroutine_enter(co, acb);
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index fbbc496..41848b2 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -157,8 +157,7 @@ static void mirror_read_complete(void *opaque, int ret)
>  return;
>  }
>  blk_aio_pwritev(s->target, op->sector_num * BDRV_SECTOR_SIZE, >qiov,
> -op->nb_sectors * BDRV_SECTOR_SIZE,
> -mirror_write_complete, op);
> +0, mirror_write_complete, op);
>  }
> 
>  static inline void mirror_clip_sectors(MirrorBlockJob *s,
> @@ -275,8 +274,7 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t 
> sector_num,
>  s->sectors_in_flight += nb_sectors;
>  trace_mirror_one_iteration(s, sector_num, nb_sectors);
> 
> -blk_aio_preadv(source, sector_num * BDRV_SECTOR_SIZE, >qiov,
> -   nb_sectors * BDRV_SECTOR_SIZE,
> +blk_aio_preadv(source, sector_num * BDRV_SECTOR_SIZE, >qiov, 0,
> mirror_read_complete, op);
>  return ret;
>  }
> -- 
> 2.5.5
> 
> 

[Qemu-devel] [PATCH] block: Avoid bogus flags during mirroring

[Eric Blake]

Commit e253f4b8 converted mirroring from sector-based bdrv_aio_*
to byte-based blk_aio_*, but failed to account for the subtle
difference in signatures (the former takes a semi-redundant length,
the latter takes a flags parameter).  Since all of our flags are
currently smaller in size than BDRV_SECTOR_SIZE, it has no ill
effects until we either perform sub-sector mirroring, or we start
asserting that no unexpected flags are set.  I found it while
testing new asserts when qemu-iotests 132 started warning about an
unknown flag 0x20.

Add an assert to help us catch any other improper flags.

Signed-off-by: Eric Blake 
---
 block/block-backend.c | 2 ++
 block/mirror.c| 6 ++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 34500e6..a5dc6e3 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -945,6 +945,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t 
offset, int bytes,
 acb->bh = NULL;
 acb->has_returned = false;

+assert(flags <= 0x1f);
+
 co = qemu_coroutine_create(co_entry);
 qemu_coroutine_enter(co, acb);

diff --git a/block/mirror.c b/block/mirror.c
index fbbc496..41848b2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -157,8 +157,7 @@ static void mirror_read_complete(void *opaque, int ret)
 return;
 }
 blk_aio_pwritev(s->target, op->sector_num * BDRV_SECTOR_SIZE, >qiov,
-op->nb_sectors * BDRV_SECTOR_SIZE,
-mirror_write_complete, op);
+0, mirror_write_complete, op);
 }

 static inline void mirror_clip_sectors(MirrorBlockJob *s,
@@ -275,8 +274,7 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t 
sector_num,
 s->sectors_in_flight += nb_sectors;
 trace_mirror_one_iteration(s, sector_num, nb_sectors);

-blk_aio_preadv(source, sector_num * BDRV_SECTOR_SIZE, >qiov,
-   nb_sectors * BDRV_SECTOR_SIZE,
+blk_aio_preadv(source, sector_num * BDRV_SECTOR_SIZE, >qiov, 0,
mirror_read_complete, op);
 return ret;
 }
-- 
2.5.5

[Qemu-devel] [PULL v2 12/15] qht: add qht-bench, a performance benchmark

[Richard Henderson]

From: "Emilio G. Cota" 

This serves as a performance benchmark as well as a stress test
for QHT. We can tweak quite a number of things, including the
number of resize threads and how frequently resizes are triggered.

A performance comparison of QHT vs CLHT[1] and ck_hs[2] using
this same benchmark program can be found here:
  http://imgur.com/a/0Bms4

The tests are run on a 64-core AMD Opteron 6376, pinning threads
to cores favoring same-socket cores. For each run, qht-bench is
invoked with:
  $ tests/qht-bench -d $duration -n $n -u $u -g $range
, where $duration is in seconds, $n is the number of threads,
$u is the update rate (0.0 to 100.0), and $range is the number
of keys.

Note that ck_hs's performance drops significantly as writes go
up, since it requires an external lock (I used a ck_spinlock)
around every write.

Also, note that CLHT instead of using a seqlock, relies on an
allocator that does not ever return the same address during the
same read-critical section. This gives it a slight performance
advantage over QHT on read-heavy workloads, since the seqlock
writes aren't there.

[1] CLHT: https://github.com/LPD-EPFL/CLHT
  https://infoscience.epfl.ch/record/207109/files/ascy_asplos15.pdf

[2] ck_hs: http://concurrencykit.org/
   http://backtrace.io/blog/blog/2015/03/13/workload-specialization/

A few of those plots are shown in text here, since that site
might not be online forever. Throughput is on Mops/s on the Y axis.

 200K keys, 0 % updates

  450 ++--+--+--+---+---+---+---+--+---+--++
  |   +  +  +   +   +   +   +  +  +N+  |
  400 ++   ---+E+ ++
  |   +++  |
  350 ++  9 ++--+--++   --+E+-+H+ ++
  | |  +H+- | -+N+    +++  |
  300 ++  8 ++ +E+ ++ -+E+  --+H+ ++
  | |  +++  | -+N+-+H+--   |
  250 ++  7 ++--+--++  +++-+E+++
  200 ++1 -+E+-+H+++
  |    qht +-E--+  |
  150 ++  -+E+clht +-H--+ ++
  |     ck +-N--+  |
  100 ++   +E+++
  ||
   50 ++   -+E+   ++
  |   +E+E+  +  +   +   +   +   +  +   +   |
0 ++--E--+--+---+---+---+---+--+---+--++
  1  8  16  24  32  40  48 56  64
Number of threads

 200K keys, 1 % updates

  350 ++--+--+--+---+---+---+---+--+---+--++
  |   +  +  +   +   +   +   +  + -+E+  |
  300 ++ -+H+ ++
  |   +E+--|
  |   9 ++--+--++  +++ |
  250 ++|  +E+   -- | -+E+++
  |   8 ++ --  ++  |
  200 ++|  +++- |  +++  ---+E+++
  |   7 ++--N--++ -+E+--   qht +-E--+  |
  | 1  +++clht +-H--+  |
  150 ++  -+E+  ck +-N--+ ++
  |    |
  100 ++   +E+++
  ||
  |-+E+|
   50 +++H+-+N++N+-+N+--  ++
  |   +E+E+  +  +   +  +N+-+N+-+N++N+-+N+  |
0 ++--E--+--+---+---+---+---+--+---+--++
  1  8  16  24  32  40  48 56  64
Number of threads

 200K keys, 20 % updates

  300 ++--+--+--+---+---+---+---+--+---+--++
  |   +  +  +   +   +   +   +  +   +   |
  |  -+H+  |
  250 ++  ++
  |   

[Qemu-devel] [PULL v2 04/15] include/processor.h: define cpu_relax()

[Richard Henderson]

From: "Emilio G. Cota" 

Taken from the linux kernel.

Reviewed-by: Sergey Fedorov 
Reviewed-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Signed-off-by: Emilio G. Cota 
Message-Id: <1465412133-3029-5-git-send-email-c...@braap.org>
Signed-off-by: Richard Henderson 
---
 include/qemu/processor.h | 30 ++
 1 file changed, 30 insertions(+)
 create mode 100644 include/qemu/processor.h

diff --git a/include/qemu/processor.h b/include/qemu/processor.h
new file mode 100644
index 000..8b25702
--- /dev/null
+++ b/include/qemu/processor.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2016, Emilio G. Cota 
+ *
+ * License: GNU GPL, version 2.
+ *   See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_PROCESSOR_H
+#define QEMU_PROCESSOR_H
+
+#include "qemu/atomic.h"
+
+#if defined(__i386__) || defined(__x86_64__)
+# define cpu_relax() asm volatile("rep; nop" ::: "memory")
+
+#elif defined(__ia64__)
+# define cpu_relax() asm volatile("hint @pause" ::: "memory")
+
+#elif defined(__aarch64__)
+# define cpu_relax() asm volatile("yield" ::: "memory")
+
+#elif defined(__powerpc64__)
+/* set Hardware Multi-Threading (HMT) priority to low; then back to medium */
+# define cpu_relax() asm volatile("or 1, 1, 1;" \
+  "or 2, 2, 2;" ::: "memory")
+
+#else
+# define cpu_relax() barrier()
+#endif
+
+#endif /* QEMU_PROCESSOR_H */
-- 
2.5.5

[Qemu-devel] [PULL v2 00/15] tb hash improvements

[Richard Henderson]

Pull v2, with the fixed ppc cpu_relax, and the two %lu fixes.
Not re-sending the rest of the patch set.


r~


The following changes since commit a93c1bdf0bd4689287094ddb2aae3dc907da3535:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20160610-1' into 
staging (2016-06-10 15:47:17 +0100)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20160611

for you to fetch changes up to 329844d4bc3d5a11f1e63938d66f74c9584c7abc:

  translate-all: add tb hash bucket info to 'info jit' dump (2016-06-11 
17:11:16 -0700)


TB hashing improvements


Emilio G. Cota (14):
  compiler.h: add QEMU_ALIGNED() to enforce struct alignment
  seqlock: remove optional mutex
  seqlock: rename write_lock/unlock to write_begin/end
  include/processor.h: define cpu_relax()
  exec: add tb_hash_func5, derived from xxhash
  tb hash: hash phys_pc, pc, and flags with xxhash
  qdist: add module to represent frequency distributions of data
  qdist: add test program
  qht: QEMU's fast, resizable and scalable Hash Table
  qht: add test program
  qht: add qht-bench, a performance benchmark
  qht: add test-qht-par to invoke qht-bench from 'check' target
  tb hash: track translated blocks with qht
  translate-all: add tb hash bucket info to 'info jit' dump

Guillaume Delbergue (1):
  qemu-thread: add simple test-and-set spinlock

 cpu-exec.c|  92 ++---
 cpus.c|  30 +-
 include/exec/exec-all.h   |   2 -
 include/exec/tb-context.h |   7 +-
 include/exec/tb-hash-xx.h |  94 ++
 include/exec/tb-hash.h|   7 +-
 include/qemu/compiler.h   |   2 +
 include/qemu/processor.h  |  30 ++
 include/qemu/qdist.h  |  63 
 include/qemu/qht.h| 183 ++
 include/qemu/seqlock.h|  14 +-
 include/qemu/thread.h |  35 ++
 tests/.gitignore  |   4 +
 tests/Makefile.include|  14 +-
 tests/qht-bench.c | 488 +++
 tests/test-qdist.c| 384 +
 tests/test-qht-par.c  |  56 
 tests/test-qht.c  | 159 +
 translate-all.c   | 131 +---
 util/Makefile.objs|   2 +
 util/qdist.c  | 395 ++
 util/qht.c| 833 ++
 22 files changed, 2893 insertions(+), 132 deletions(-)
 create mode 100644 include/exec/tb-hash-xx.h
 create mode 100644 include/qemu/processor.h
 create mode 100644 include/qemu/qdist.h
 create mode 100644 include/qemu/qht.h
 create mode 100644 tests/qht-bench.c
 create mode 100644 tests/test-qdist.c
 create mode 100644 tests/test-qht-par.c
 create mode 100644 tests/test-qht.c
 create mode 100644 util/qdist.c
 create mode 100644 util/qht.c

Re: [Qemu-devel] [PULL 00/15] tb hash improvements

[Richard Henderson]

On 06/10/2016 09:41 AM, Peter Maydell wrote:

On 10 June 2016 at 17:34, Emilio G. Cota  wrote:

On Fri, Jun 10, 2016 at 16:33:10 +0100, Peter Maydell wrote:

Fails to build on ppc64be :-(

In file included from /home/pm215/qemu/include/qemu/thread.h:4:0,
 from /home/pm215/qemu/include/block/aio.h:20,
 from /home/pm215/qemu/include/block/block.h:4,
 from /home/pm215/qemu/include/monitor/monitor.h:6,
 from /home/pm215/qemu/trace/control.c:23:
/home/pm215/qemu/include/qemu/processor.h:24:35: error: expected
identifier or ‘(’ before string constant
   "or 2, 2, 2;" ::: "memory")


On Fri, Jun 10, 2016 at 16:57:19 +0100, Peter Maydell wrote:

Also fails trying to build a test on 32-bit:

/home/petmay01/qemu/tests/qht-bench.c: In function 'pr_params':
/home/petmay01/qemu/tests/qht-bench.c:270:5: error: format '%zu'
expects argument of type 'size_t', but argument 2 has type 'long
unsigned int' [-Werror=format=]
 printf(" lookup range:  %zu\n", lookup_range);


Can you please test again after applying the appended delta?


I will test.

For the PPC asm, is it not just wanting the "\n" between instructions?
Shouldn't be necessary to use two separate asm() lines...


It has a ; between instructions.  That's not the problem.

What it needs is a \ to continue the macro to the second line.

I've just finished a build on ppc64 with that one character fix.


r~

Re: [Qemu-devel] [PATCH 3/5] block: Switch transfer length bounds to byte-based

[Eric Blake]

On 06/07/2016 06:45 AM, Kevin Wolf wrote:
> Am 03.06.2016 um 19:03 hat Eric Blake geschrieben:
>> Sector-based limits are awkward to think about; in our on-going
>> quest to move to byte-based interfaces, convert max_transfer_length
>> and opt_transfer_length.  Rename them (dropping the _length suffix)
>> so that the compiler will help us catch the change in semantics
>> across any rebased code.  Improve the documentation, and guarantee
>> that blk_get_max_transfer() returns a non-zero value.  Use unsigned
>> values, so that we don't have to worry about negative values and
>> so that bit-twiddling is easier; however, we are still constrained
>> by 2^31 of signed int in most APIs.
>>
>> Of note: the iscsi calculations use a 64-bit number internally,
>> but the final result is guaranteed to fit in 32 bits.  NBD is
>> fixed to advertise the maximum length of 32M that the qemu and
>> kernel servers enforce, rather than a number of sectors that
>> would overflow int when converted back to bytes.  scsi-generic
>> now advertises a maximum always, rather than just when the
>> underlying device advertised a maximum.
>>
>> Signed-off-by: Eric Blake 
> 
>> @@ -1177,7 +1176,7 @@ static int coroutine_fn 
>> bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
>>
>>  if (ret == -ENOTSUP) {
>>  /* Fall back to bounce buffer if write zeroes is unsupported */
>> -int max_xfer_len = MIN_NON_ZERO(bs->bl.max_transfer_length,
>> +int max_xfer_len = MIN_NON_ZERO(bs->bl.max_transfer,
>>  MAX_WRITE_ZEROES_BOUNCE_BUFFER);
> 
> You could consider renaming the variable to max_transfer to keep it
> consistent with the BlockLimits field name and to make it even more
> obvious that you converted all uses.

Good idea.

>> @@ -1706,13 +1708,14 @@ static void iscsi_refresh_limits(BlockDriverState 
>> *bs, Error **errp)
>>   * iscsi_open(): iscsi targets don't change their limits. */
>>
>>  IscsiLun *iscsilun = bs->opaque;
>> -uint32_t max_xfer_len = iscsilun->use_16_for_rw ? 0x : 0x;
>> +uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0x : 0x;
>>
>>  if (iscsilun->bl.max_xfer_len) {
>>  max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len);
>>  }
>>
>> -bs->bl.max_transfer_length = sector_limits_lun2qemu(max_xfer_len, 
>> iscsilun);
>> +bs->bl.max_transfer = MIN(BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS,
>> +  max_xfer_len * iscsilun->block_size);
> 
> Why don't you simply use 0 when you defined it as no limit?
> 
> If we make some drivers put 0 there and others INT_MAX, chances are that
> we'll introduce places where we fail to handle 0 correctly.

So if I'm understanding correctly, we want something like:

if (max_xfer_len * iscsilun->block_size > INT_MAX) {
bs->bl.max_transfer = 0;
} else {
bs->bl.max_transfer = max_xfer_len * iscsilun->block_size;
}

and make sure that 0 continues to mean 'no signed 32-bit limit'.

>> +++ b/block/nbd.c
>> @@ -363,7 +363,7 @@ static int nbd_co_flush(BlockDriverState *bs)
>>  static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
>>  {
>>  bs->bl.max_discard = UINT32_MAX >> BDRV_SECTOR_BITS;
>> -bs->bl.max_transfer_length = UINT32_MAX >> BDRV_SECTOR_BITS;
>> +bs->bl.max_transfer = NBD_MAX_BUFFER_SIZE;
>>  }
> 
> This introduces a semantic difference and should therefore be a separate
> patch. Here, it should become UINT32_MAX through mechanical conversion.
> 
> Or actually, it can't because that's not a power of two. So maybe have
> the NBD patch first...

Will split.  I don't see any problem with a max_transfer that is not a
power of two, as long as it is a multiple of request_alignment (iscsi is
a case in point - the max_transfer can be 0x blocks).

> 
>>  static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index ce2e20f..f3bd5ef 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -752,7 +752,7 @@ static void raw_refresh_limits(BlockDriverState *bs, 
>> Error **errp)
>>  if (S_ISBLK(st.st_mode)) {
>>  int ret = hdev_get_max_transfer_length(s->fd);
>>  if (ret >= 0) {
>> -bs->bl.max_transfer_length = ret;
>> +bs->bl.max_transfer = ret << BDRV_SECTOR_BITS;
> 
> I assume that we don't care about overflows here because in practice the
> values are very low? Should we check (or maybe better just cap) it
> anyway?

Will add a check.

>> @@ -391,8 +391,9 @@ void virtio_blk_submit_multireq(BlockBackend *blk, 
>> MultiReqBuffer *mrb)
>>  return;
>>  }
>>
>> -max_xfer_len = blk_get_max_transfer_length(mrb->reqs[0]->dev->blk);
>> -max_xfer_len = MIN_NON_ZERO(max_xfer_len, BDRV_REQUEST_MAX_SECTORS);
>> +max_xfer_len = blk_get_max_transfer(mrb->reqs[0]->dev->blk);
>> +assert(max_xfer_len &&
>> +   

Re: [Qemu-devel] [PATCH 2/5] block: Honor flags during bdrv_aligned_preadv()

[Eric Blake]

On 06/07/2016 06:12 AM, Kevin Wolf wrote:
> Am 03.06.2016 um 19:03 hat Eric Blake geschrieben:
>> Not that we pass any flags during reads yet, but we may want to
>> support BDRV_REQ_FUA on reads in the future.  So don't throw
>> away the input flags.
>>
>> Signed-off-by: Eric Blake 
> 
> Do we want to pass flags to bdrv_co_do_copy_on_readv(), too? I guess we
> would use them for the preadv call there, but continue to use 0 as the
> pwritev flags.

What about BDRV_REQ_MAY_UNMAP if bdrv_co_do_copy_on_readv() detects a
block of zeroes?

This just got a lot trickier, so I think my short-term solution is to
just assert(!flags) and add a comment that whoever implements flags on
read has to solve the issue at that time, so that I'm not stalling the
rest of this series on thinking about things that don't matter yet.

> 
> We may also want to introduce a .supported_read_flags, but perhaps this
> is not something to do in this patch.

Yeah, I think we'll want that, at the point where we start worrying
about read flags.

> And we don't even use
> .supported_write_flags for drivers that have a native .bdrv_co_pwritev.
> Of course, that's even less related to this patch. I guess I should send
> a fix for that.

Thanks.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] Call cmp with "-s" instead of "--quiet"

[Eric Blake]

On 06/11/2016 11:15 AM, Sean Bruno wrote:
> 
> 
> --quiet isn't available on FreeBSD and doesn't seem to be used in
> other places.  This fixes some non-fatal build errors on FreeBSD.  The
> use of "-s" over "--quiet" seems to be the preferred use in the Makefile.
> 
> diff --git a/Makefile b/Makefile
> index ed4032a..a7a356a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -185,7 +185,7 @@ qemu-version.h: FORCE
> printf '""\n'; \
> fi; \
> fi) > $@.tmp)
> -   $(call quiet-command, cmp --quiet $@ $@.tmp || mv $@.tmp $@)
> +   $(call quiet-command, cmp -s $@ $@.tmp || mv $@.tmp $@)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] hw/sh4/sh_pci.c: Use ldl_le_p() and stl_le_p()

[Eric Blake]

On 06/10/2016 10:10 AM, Peter Maydell wrote:
> Use ldl_le_p() and stl_le_p() instead of le32_to_cpup() and
> cpu_to_le32w(); the former handle misaligned addresses and don't
> need casts, and the latter are deprecated.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/sh4/sh_pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] qemu -nographics option

[David Craven]

Hello Marcin

Reordering the console statements worked. The reason I had so many consoles
in the kernel command line was because it was recommended in
redirect-qemu-window-output-to-terminal-running-qemu
.
I wasn't aware that ordering was important.

Thank you!
David

On Sat, Jun 11, 2016 at 9:52 PM, mar.krzeminski 
wrote:

> Hello,
>
> Have you try to add chardev device?
>
> -chardev stdio,mux=on,id=terminal -serial chardev:terminal
>
> I am also not sure if you so many consoles in kernel command line,
> since you do not want to use any graphic mode.
>
> Regards,
> Marcin
>
>
> W dniu 11.06.2016 o 16:22, David Craven pisze:
>
> Hello qemu devs,
>>
>> I previously asked this question on stack overflow but didn't get an
>> answer, so I thought I'd ask here:
>>
>> Stdio doesn't show in qemu with the -nographic option.
>> This is my qemu invocation. It prints hello world when
>> using `-serial stdio` but only displays kernel output when
>> using `-nographic`. (From the docs I gather these two are
>> mutually exclusive options)
>>
>> ```
>>  qemu-system-arm \
>>  -kernel zImage \
>>  -append "\
>>  console=ttyAMA0,115200 console=tty highres=off console=ttyS0
>> \
>>  rdinit=/bin/static-hello \
>>  panic=1" \
>>  -initrd rootfs.cpio.gz \
>>  -dtb vexpress-v2p-ca9.dtb \
>>  -M vexpress-a9 \
>>  -m 512 \
>>  -nographic \
>>  -no-reboot
>> ```
>>
>> How do I get this to work with the `-nographic` option?
>>
>> David
>>
>>
>>
>

Re: [Qemu-devel] [PATCH] block: drop support for using qcow[2] encryption with system emulators

[Eric Blake]

On 06/10/2016 09:32 AM, Daniel P. Berrange wrote:
> Back in the 2.3.0 release we declared qcow[2] encryption as
> deprecated, warning people that it would be removed in a future
> release.
> 
>   commit a1f688f4152e65260b94f37543521ceff8bfebe4
>   Author: Markus Armbruster 
>   Date:   Fri Mar 13 21:09:40 2015 +0100
> 
> block: Deprecate QCOW/QCOW2 encryption
> 
> The code still exists today, but by a (happy?) accident we entirely
> broke the ability to use qcow[2] encryption in the system emulators
> in the 2.4.0 release due to
> 
>   commit 8336aafae1451d54c81dd2b187b45f7c45d2428e
>   Author: Daniel P. Berrange 
>   Date:   Tue May 12 17:09:18 2015 +0100
> 
> qcow2/qcow: protect against uninitialized encryption key
> 
> This commit was designed to prevent future coding bugs which
> might cause QEMU to read/write data on an encrypted block
> device in plain text mode before a decryption key is set.
> 
> It turns out this preventative measure was a little too good,
> because we already had a long standing bug where QEMU read
> encrypted data in plain text mode during system emulator
> startup, in order to guess disk geometry:

Interesting analysis.


> So rather than fix the crash, and backport it to stable
> releases, just go ahead with what we have warned users about
> and disable any use of qcow2 encryption in the system
> emulators. qemu-img/qemu-io/qemu-nbd are still able to access
> qcow2 encrypted images for the sake of data conversion.
> 
> In the future, qcow2 will gain support for the alternative
> luks format, but when this happens it'll be using the
> '-object secret' infrastructure for gettings keys, which
> avoids this problematic scenario entirely.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/qcow.c   | 11 +++
>  block/qcow2.c  | 11 +++
>  tests/qemu-iotests/087.out | 12 ++--
>  3 files changed, 16 insertions(+), 18 deletions(-)


> +++ b/block/qcow.c
> @@ -162,10 +162,13 @@ static int qcow_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  if (s->crypt_method_header) {
>  if (bdrv_uses_whitelist() &&
>  s->crypt_method_header == QCOW_CRYPT_AES) {
> -error_report("qcow built-in AES encryption is deprecated");
> -error_printf("Support for it will be removed in a future 
> release.\n"
> - "You can use 'qemu-img convert' to switch to an\n"
> - "unencrypted qcow image, or a LUKS raw image.\n");
> +error_setg(errp,
> +   "Use of AES-CBC encrypted qcow images is no longer "
> +   "supported in system emulators. You can use "
> +   "'qemu-img convert' to convert your image to use "
> +   "the LUKS format instead.");

error_setg() should not end in '.'.  Better would be:

error_setg(errp, "Use of AES-CBC encrypted qcow images is not supported");
error_append_hint(errp, "You can use 'qemu-img convert'... instead.\n");

> +++ b/block/qcow2.c
> @@ -968,10 +968,13 @@ static int qcow2_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  if (s->crypt_method_header) {
>  if (bdrv_uses_whitelist() &&
>  s->crypt_method_header == QCOW_CRYPT_AES) {
> -error_report("qcow2 built-in AES encryption is deprecated");
> -error_printf("Support for it will be removed in a future 
> release.\n"
> - "You can use 'qemu-img convert' to switch to an\n"
> - "unencrypted qcow2 image, or a LUKS raw image.\n");
> +error_setg(errp,
> +   "Use of AES-CBC encrypted qcow2 images is no longer "
> +   "supported in system emulators. You can use "
> +   "'qemu-img convert' to convert your image to use "
> +   "the LUKS format instead.");

and again.

> +ret = -ENOSYS;
> +goto fail;
>  }
>  
>  bs->encrypted = 1;
> diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> index 055c553..99853c5 100644
> --- a/tests/qemu-iotests/087.out
> +++ b/tests/qemu-iotests/087.out
> @@ -42,22 +42,14 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> encryption=on
>  Testing: -S
>  QMP_VERSION
>  {"return": {}}
> -IMGFMT built-in AES encryption is deprecated
> -Support for it will be removed in a future release.
> -You can use 'qemu-img convert' to switch to an
> -unencrypted IMGFMT image, or a LUKS raw image.
> -{"error": {"class": "GenericError", "desc": "blockdev-add doesn't support 
> encrypted devices"}}
> +{"error": {"class": "GenericError", "desc": "Use of AES-CBC encrypted qcow2 
> images is no longer supported in system emulators. You can use 'qemu-img 
> convert' to convert your image to use the LUKS format instead."}}

And this will need tweaking to match.

I'm 

Re: [Qemu-devel] [PATCH] hw/usb/dev-network.c: Use ldl_le_p() and stl_le_p()

[Eric Blake]

On 06/10/2016 09:37 AM, Peter Maydell wrote:
> Use stl_le_p() and ldl_le_p() to read and write data from
> buffers, rather than using pointer casts and cpu_to_le32()
> for writes and le32_to_cpup() for reads. This:
>  * avoids lots of casts
>  * works even if the buffer isn't as aligned as the host would like
>  * avoids using the *_to_cpup() functions which we want to get rid of
> 
> Note that there may still be some places where a pointer from the
> guest is cast to a pointer to a host structure; these would also
> have to be changed for the device to work on a host CPU which
> enforces alignment restrictions.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/usb/dev-network.c | 63 
> +---
>  1 file changed, 30 insertions(+), 33 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC 02/10] softmmu_llsc_template.h: Move to multi-threading

[Sergey Fedorov]

On 10/06/16 19:15, Alex Bennée wrote:
> Sergey Fedorov  writes:
>
>> On 26/05/16 19:35, Alvise Rigo wrote:
>>> Using tcg_exclusive_{lock,unlock}(), make the emulation of
>>> LoadLink/StoreConditional thread safe.
>>>
>>> During an LL access, this lock protects the load access itself, the
>>> update of the exclusive history and the update of the VCPU's protected
>>> range.  In a SC access, the lock protects the store access itself, the
>>> possible reset of other VCPUs' protected range and the reset of the
>>> exclusive context of calling VCPU.
>>>
>>> The lock is also taken when a normal store happens to access an
>>> exclusive page to reset other VCPUs' protected range in case of
>>> collision.
>> I think the key problem here is that the load in LL helper can race with
>> a concurrent regular fast-path store. 
(snip)
> I think this can be fixed using async_safe_run_on_cpu and tweaking the
> ldlink helper.
>
>   * Change the helper_ldlink call
> - pass it offset-of(cpu->reg[n]) so it can store result of load
> - maybe pass it next-pc (unless there is some other way to know)
>
>   vCPU runs until the ldlink instruction occurs and jumps to the helper
>
>   * Once in the helper_ldlink
> - queue up an async helper function with info of offset
> - cpu_loop_exit_restore(with next PC)
>
>   vCPU the issued the ldlink exits immediately, waits until all vCPUs are
>   out of generated code.
>
>   * Once in helper_ldlink async helper
> - Everything at this point is quiescent, no vCPU activity
> - Flush all TLBs/set flags
> - Do the load from memory, store directly into cpu->reg[n]
>
> The key thing is once we are committed to load in the async helper
> nothing else can get in the way. Any stores before we are in the helper
> happen as normal, once we exit the async helper all potential
> conflicting stores will slow path.
>
> There is a little messing about in knowing the next PC which is simple
> in the ARM case but gets a bit more complicated for architectures that
> have deferred jump slots. I haven't looked into this nit yet.


Hmm, this looks pretty similar to what linux-user code actually does,
e.g. in do_strex(). Just restarting the LL instruction as Alvise
suggests may well be an easier approach (or may not).

Kind regards,
Sergey

Re: [Qemu-devel] qemu -nographics option

[mar.krzeminski]

Hello,

Have you try to add chardev device?

-chardev stdio,mux=on,id=terminal -serial chardev:terminal

I am also not sure if you so many consoles in kernel command line,
since you do not want to use any graphic mode.

Regards,
Marcin


W dniu 11.06.2016 o 16:22, David Craven pisze:

Hello qemu devs,

I previously asked this question on stack overflow but didn't get an
answer, so I thought I'd ask here:

Stdio doesn't show in qemu with the -nographic option.
This is my qemu invocation. It prints hello world when
using `-serial stdio` but only displays kernel output when
using `-nographic`. (From the docs I gather these two are
mutually exclusive options)

```
 qemu-system-arm \
 -kernel zImage \
 -append "\
 console=ttyAMA0,115200 console=tty highres=off console=ttyS0 \
 rdinit=/bin/static-hello \
 panic=1" \
 -initrd rootfs.cpio.gz \
 -dtb vexpress-v2p-ca9.dtb \
 -M vexpress-a9 \
 -m 512 \
 -nographic \
 -no-reboot
```

How do I get this to work with the `-nographic` option?

David

Re: [Qemu-devel] [RFC] hw/i386: Composite Bus and PCI device

[David Kiarie]

On Fri, Jun 10, 2016 at 8:30 AM, Jan Kiszka  wrote:
> On 2016-06-08 17:25, Eduardo Habkost wrote:
>> On Wed, Jun 08, 2016 at 01:00:32PM +0300, David Kiarie wrote:
>>> Sample composite SysBus and PCI device similar to AMD IOMMU setup
>>>
>>> Signed-off-by: David Kiarie 
>>> ---
>>>  hw/i386/compositedevice.c | 113 
>>> ++
>>>  1 file changed, 113 insertions(+)
>>>  create mode 100644 hw/i386/compositedevice.c
>>
>> The filename is very generic (hw/i386/compositedevice.c), but it
>> has lots of AMD-specific names.
>>
>> Is your plan to provide generic helpers for implementing
>> SysBus+PCI devices, or is it going to be inside a source file
>> specific for AMD IOMMU?
>
> As far as I understood - David, correct me - this patch is more of a
> simplified demonstrator of the architecture to be applied on the actual
> AMD IOMMU code. It is not for merge.

Right.

>
> Jan
>
>

[Qemu-devel] [PATCH] Call cmp with "-s" instead of "--quiet"

[Sean Bruno]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512



- --quiet isn't available on FreeBSD and doesn't seem to be used in
other places.  This fixes some non-fatal build errors on FreeBSD.  The
use of "-s" over "--quiet" seems to be the preferred use in the Makefile.

diff --git a/Makefile b/Makefile
index ed4032a..a7a356a 100644
- --- a/Makefile
+++ b/Makefile
@@ -185,7 +185,7 @@ qemu-version.h: FORCE
printf '""\n'; \
fi; \
fi) > $@.tmp)
- -   $(call quiet-command, cmp --quiet $@ $@.tmp || mv $@.tmp $@)
+   $(call quiet-command, cmp -s $@ $@.tmp || mv $@.tmp $@)

 config-host.h: config-host.h-timestamp
 config-host.h-timestamp: config-host.mak
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQF8BAEBCgBmBQJXXEcgXxSAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRCQUFENDYzMkU3MTIxREU4RDIwOTk3REQx
MjAxRUZDQTFFNzI3RTY0AAoJEBIB78oecn5kpWgIAI/ZzRYWUWNYXF9D8NuvPlub
Arh2F7qBBvukaMra5fVQwJcNji82WdEHmClsV5782rPM19LC7QVqiiga494WFbQ2
mGWVJF0w9w0dA0lCz65mlWwqX0U/u0No8Z/RMoBtublTvbHtLfy/OUAMJrV940X2
EbPu0x/0vBlUCfgf2KUNcs6O6lt9AUJFvrSzeTJgLNHml7hchyHVMQujMv/2YggH
4TscRWswBcmikP8Rorxf59mBQx/om2n3omgnWxlAuvrgqm0ief42pYYkyIqTaPr0
C1dubmrV8ArcMWlGStUAhV2W1ibsph3ZSgoUuHqkpv5iz5BFizIqS+EvWfo1OBM=
=x1ai
-END PGP SIGNATURE-

[Qemu-devel] qemu -nographics option

[David Craven]

Hello qemu devs,

I previously asked this question on stack overflow but didn't get an
answer, so I thought I'd ask here:

Stdio doesn't show in qemu with the -nographic option.
This is my qemu invocation. It prints hello world when
using `-serial stdio` but only displays kernel output when
using `-nographic`. (From the docs I gather these two are
mutually exclusive options)

```
qemu-system-arm \
-kernel zImage \
-append "\
console=ttyAMA0,115200 console=tty highres=off console=ttyS0 \
rdinit=/bin/static-hello \
panic=1" \
-initrd rootfs.cpio.gz \
-dtb vexpress-v2p-ca9.dtb \
-M vexpress-a9 \
-m 512 \
-nographic \
-no-reboot
```

How do I get this to work with the `-nographic` option?

David

Re: [Qemu-devel] [RFC] Allow AMD IOMMU to have both SysBusDevice and PCIDevice properties.

[David Kiarie]

On Thu, Jun 9, 2016 at 4:23 PM, Marcel Apfelbaum  wrote:
> On 06/07/2016 10:12 PM, Eduardo Habkost wrote:
>>
>> Hi,
>>
>
> [...]
>
>> [...]
>>>
>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>> index 04aae89..431eaed 100644
>>> --- a/hw/i386/pc_q35.c
>>> +++ b/hw/i386/pc_q35.c
>>> @@ -281,6 +281,7 @@ static void pc_q35_machine_options(MachineClass *m)
>>>   m->default_machine_opts = "firmware=bios-256k.bin";
>>>   m->default_display = "std";
>>>   m->no_floppy = 1;
>>> +m->has_dynamic_sysbus = true;
>>
>>
>> Why is this needed? Is it possible to do this change before
>> adding the iommu code?  Can this be done in a separate patch that
>> documents why it should be changed and why it is safe to set it
>> to true?
>>
>>>
>
> Hi Eduardo,
>
> I also have this change as part of '[PATCH v2 0/3] enable iommu with
> -device'.
>
> Please see:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg374644.html

This change is actually yours. My code is depends on your code.

>
>
> Thanks,
> Marcel

Re: [Qemu-devel] [Qemu-block] [PATCH] mirror: add target-zeroed flag

[Denis V. Lunev]

On 06/10/2016 07:59 PM, Stefan Hajnoczi wrote:

On Tue, Jun 07, 2016 at 07:30:19PM +0300, Vladimir Sementsov-Ogievskiy wrote:

On 03.06.2016 18:45, Denis V. Lunev wrote:

On 06/03/2016 06:06 PM, Eric Blake wrote:

On 06/03/2016 08:05 AM, Vladimir Sementsov-Ogievskiy wrote:

Add target-zeroed flag to allow user specify that target is already
zeroed. With this flag set zeroes which was in source before mirror
start will not be copyed.

With this flag set, any runs of zeroes in the source before the mirror
starts will not be copied.


Without this libvirt migration of empty disk takes too long time.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

I've tested it with
time virsh migrate --live test qemu+ssh://other_node/system
--copy-storage-all

Presumably with a libvirt patch to turn on the optional flag.

I'm not sure I like this patch.  Libvirt uses NBD to implement
--copy-storage-all, I think we're better off improving NBD to
automatically handle sparse writes, than we are to add a one-off hack
that requires libvirt to change.  That is, once NBD is smarter, the copy
will be faster without needing a tweak.  And we ARE working on making
NBD smarter (one of my goals for the 2.7 release is to get all the
sparse file additions to NBD implemented)

That said, I'll still review it.

this is not enough, definitely.

There is a problem that mirror_iteration code sleeps even for
not read zeroes (and this IS slow). Moreover, even sending sparcified
zeroes takes a lot of time for round trips.

We have started with that and spent a lot of time trying to improve
the situation.

Also, as a side note, the QCOW2 file on a source and a target will
be different without the flag - original image has empty blocks,
target image will have blocks explicitly marked with zeroes.

Though this is a matter of taste... For us this approach is the simplest.

Den

Hey, what do think about it? Don't we forget somebody to be cc'ed?

Jeff Cody  maintains block jobs.  This patch should go
through him.

Stefan

thank you.

I think I have a better patch for this without additional flag.
I have used approach similar one in qemu-img. I am going
to resubmit more with driver-mirror stuff at the beginning
of next week.

Den

Re: [Qemu-devel] [PATCH V9 0/9] Xilinx DisplayPort.

[KONRAD Frederic]

Le 09/06/2016 à 20:05, Peter Maydell a écrit :

On 7 June 2016 at 22:39, Alistair Francis  wrote:

On Tue, Jun 7, 2016 at 1:30 PM,   wrote:

From: KONRAD Frederic 


Hey Peter,

These are all reviewed by Xilinx, this is ready to merge from our point of view.


This series breaks 'make check'. Specifically, the patch
"i2c: implement broadcast write" causes:

(cd build/x86 && QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm
QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM %
255 + 1))} gtester -k --verbose -m=quick tests/ds1338-test)
TEST: tests/ds1338-test... (pid=22635)
  /arm/ds1338/tx-rx:
Broken pipe
FAIL
GTester: last random seed: R02Se51ed4e024b0f59ef23e564a2a28bb83
(pid=22641)
FAIL: tests/ds1338-test

thanks
-- PMM



Hi Peter,

Yeah sorry I found the bug I'll resend.
BTW this doesn't happen with `make check` aarch64, is that normal?

Thanks,
Fred

[Qemu-devel] [Bug 1589564] Re: qemu/hw/scsi/scsi-disk.c:2741: possible missing break ?

[dcb]

>I am currently not able to reproduce the problem with the format strings ... 
>how did you get them? Which compiler (and version) did you use?

I used a static analyser for C & C++ called cppcheck. It is available
from sourceforge. I find it very useful.

I think gcc might be able to reproduce these problems with one of the
higher warning levels. -Wformat=2 springs to mind, but a check of the gcc
documentation around -Wformat will give more accurate guidance.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1589564

Title:
  qemu/hw/scsi/scsi-disk.c:2741: possible missing break ?

Status in QEMU:
  New

Bug description:
  qemu/hw/scsi/scsi-disk.c:2741] -> [qemu/hw/scsi/scsi-disk.c:2745]: (warning) 
Variable 'cdb1' is reassigned a value before the old one has been used. 
'break;' missing?
  qemu/hw/scsi/scsi-disk.c:2742] -> [qemu/hw/scsi/scsi-disk.c:2746]: (warning) 
Variable 'group_number' is reassigned a value before the old one has been used. 
'break;' missing?

  Source code is

  case 1:
  /* 10-byte CDB.  */
  r->cdb1 = req->cmd.buf[1];
  r->group_number = req->cmd.buf[6];
  case 4:
  /* 12-byte CDB.  */

  Also,

  [qemu/hw/scsi/scsi-disk.c:2063]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2066]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2069]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2083]: (warning) %lu in format string (no. 2) 
requires 'unsigned long' but the argument type is 'signed long'.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1589564/+subscriptions

[Qemu-devel] [Bug 568053] Re: requires MSYS coreutils ext sub-package to build on Windows

[T. Huth]

signrom.sh has been removed/rewritten by this commit:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=0d6b9cc7420dd2d531b48508f0d4

** Changed in: qemu
   Status: Confirmed => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/568053

Title:
  requires MSYS coreutils ext sub-package to build on Windows

Status in QEMU:
  Fix Released

Bug description:
  When I try to build QEMU on Windows without the MSYS coreutils ext
  sub-package installed, the build fails because it cannot find dd.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/568053/+subscriptions

[Qemu-devel] [Bug 1589564] Re: qemu/hw/scsi/scsi-disk.c:2741: possible missing break ?

[T. Huth]

I am currently not able to reproduce the problem with the format strings
... how did you get them? Which compiler (and version) did you use?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1589564

Title:
  qemu/hw/scsi/scsi-disk.c:2741: possible missing break ?

Status in QEMU:
  New

Bug description:
  qemu/hw/scsi/scsi-disk.c:2741] -> [qemu/hw/scsi/scsi-disk.c:2745]: (warning) 
Variable 'cdb1' is reassigned a value before the old one has been used. 
'break;' missing?
  qemu/hw/scsi/scsi-disk.c:2742] -> [qemu/hw/scsi/scsi-disk.c:2746]: (warning) 
Variable 'group_number' is reassigned a value before the old one has been used. 
'break;' missing?

  Source code is

  case 1:
  /* 10-byte CDB.  */
  r->cdb1 = req->cmd.buf[1];
  r->group_number = req->cmd.buf[6];
  case 4:
  /* 12-byte CDB.  */

  Also,

  [qemu/hw/scsi/scsi-disk.c:2063]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2066]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2069]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2083]: (warning) %lu in format string (no. 2) 
requires 'unsigned long' but the argument type is 'signed long'.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1589564/+subscriptions

[Qemu-devel] [Bug 1589564] Re: qemu/hw/scsi/scsi-disk.c:2741: possible missing break ?

[T. Huth]

The issue with the missing break has been fixed here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ed45cae39117d41315541

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1589564

Title:
  qemu/hw/scsi/scsi-disk.c:2741: possible missing break ?

Status in QEMU:
  New

Bug description:
  qemu/hw/scsi/scsi-disk.c:2741] -> [qemu/hw/scsi/scsi-disk.c:2745]: (warning) 
Variable 'cdb1' is reassigned a value before the old one has been used. 
'break;' missing?
  qemu/hw/scsi/scsi-disk.c:2742] -> [qemu/hw/scsi/scsi-disk.c:2746]: (warning) 
Variable 'group_number' is reassigned a value before the old one has been used. 
'break;' missing?

  Source code is

  case 1:
  /* 10-byte CDB.  */
  r->cdb1 = req->cmd.buf[1];
  r->group_number = req->cmd.buf[6];
  case 4:
  /* 12-byte CDB.  */

  Also,

  [qemu/hw/scsi/scsi-disk.c:2063]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2066]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2069]: (warning) %lu in format string (no. 1) 
requires 'unsigned long' but the argument type is 'signed long'.
  [qemu/hw/scsi/scsi-disk.c:2083]: (warning) %lu in format string (no. 2) 
requires 'unsigned long' but the argument type is 'signed long'.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1589564/+subscriptions

[Qemu-devel] [Bug 391879] Re: migrate exec ignores exit status

[T. Huth]

Patch has been included here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=41ef56e61153d7bd27d34a63463
So I assume this should be working now.

** Changed in: qemu
   Status: In Progress => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/391879

Title:
  migrate exec ignores exit status

Status in QEMU:
  Fix Released
Status in qemu-kvm package in Ubuntu:
  Won't Fix

Bug description:
  Binary package hint: kvm

  Using

migrate "exec:cat > foo; false"

  in the monitor results in the state of the VM being written to foo, as
  expected, and the VM then being stopped. This is surprising, as I
  think it stands to reason that in case of a failed migrate-exec
  process, which is what a non-zero exit status implies to me, the VM
  should continue.

  == Version information

  $ lsb_release -rd
  Description:  Ubuntu 9.04
  Release:  9.04

  $ apt-cache policy kvm
  kvm:
Installed: 1:84+dfsg-0ubuntu11
Candidate: 1:84+dfsg-0ubuntu11
Version table:
   *** 1:84+dfsg-0ubuntu11 0
  500 http://gb.archive.ubuntu.com jaunty/main Packages
  100 /var/lib/dpkg/status

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/391879/+subscriptions

[Qemu-devel] [Bug 1555076] Re: Qemu 2.5 dont start with sdl, gl=on or gtk, gl=on

[T. Huth]

Fix has been pulled into the QEMU git repository:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=2c2311c5451f4555e850772

** Changed in: qemu
   Status: New => Fix Committed

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1555076

Title:
  Qemu 2.5 dont start with sdl,gl=on or gtk,gl=on

Status in QEMU:
  Fix Committed

Bug description:
  with this config line 
   qemu-system-i386 -m 2047 -hda /dev/sda3 -display sdl,gl=on -sdl -vga virtio 
-cdrom xenial-desktop-i386.iso 

  
  i have this exit

  ERROR:ui/console-gl.c:95:surface_gl_create_texture: code should not be
  reached

  same is i use this:

  qemu-system-i386 -m 2047 -hda /dev/sda3 -display gtk,gl=on -sdl -vga virtio 
-cdrom xenial-desktop-i386.iso 
  ERROR:ui/console-gl.c:95:surface_gl_create_texture: code should not be reached

  
  My Os i Debian Jessie  on P5020 PPC64 4GB ram GPU RadeonHD .
  Configure gave me gl ok, sdl ok , Virtio and Virgl OK .

  My Mesa are the 11.3 dev ... the same issue was found on oldest and
  stable release of mesa .

  OpenGL vendor string: X.Org
  OpenGL renderer string: Gallium 0.4 on AMD TURKS (DRM 2.43.0)
  OpenGL version string: 2.1 Mesa 11.3.0-devel (git-3146014)
  OpenGL shading language version string: 1.30

  
  OpenGL ES profile version string: OpenGL ES 2.0 Mesa 11.3.0-devel 
(git-3146014)
  OpenGL ES profile shading language version string: OpenGL ES GLSL ES 1.0.16

  
  Thanks 
  Luigi

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1555076/+subscriptions

[Qemu-devel] [Bug 1581796] Re: console-gl.c:96:surface_gl_create_texture:code should not be reached

[luigiburdo]

Thomas,
thank you very much
for your fix

Luigi

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1581796

Title:
  console-gl.c:96:surface_gl_create_texture:code should not be reached

Status in QEMU:
  Fix Committed

Bug description:
  Facing this if i enable gtk,gl option same is with sd2 gl options.

  PowerPc P5020 4gb ram Ubuntu Mate 16:04

  tested on
  RadeonSi 7750HD 2gb ddr3
  r600 6570 2gb ddr3

  Masa 11.2.2 and 11.3 dev
  same issue come on swrast mode

  Thanks
  Luigi

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1581796/+subscriptions

[Qemu-devel] [Bug 1581796] Re: console-gl.c:96:surface_gl_create_texture:code should not be reached

[T. Huth]

Fix has been pulled into the QEMU git repository:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=2c2311c5451f4555e850772

** Changed in: qemu
   Status: New => Fix Committed

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1581796

Title:
  console-gl.c:96:surface_gl_create_texture:code should not be reached

Status in QEMU:
  Fix Committed

Bug description:
  Facing this if i enable gtk,gl option same is with sd2 gl options.

  PowerPc P5020 4gb ram Ubuntu Mate 16:04

  tested on
  RadeonSi 7750HD 2gb ddr3
  r600 6570 2gb ddr3

  Masa 11.2.2 and 11.3 dev
  same issue come on swrast mode

  Thanks
  Luigi

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1581796/+subscriptions

Re: [Qemu-devel] [PATCH RFC 07/16] qom/cpu: make nr-cores, nr-threads real properties

[Thomas Huth]

On 10.06.2016 19:40, Andrew Jones wrote:
> Signed-off-by: Andrew Jones 
> ---
>  qom/cpu.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 751e992de8823..024cda3eb98c8 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -28,6 +28,7 @@
>  #include "exec/log.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/qdev-properties.h"
>  
>  bool cpu_exists(int64_t id)
>  {
> @@ -342,6 +343,12 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu)
>  return cpu->cpu_index;
>  }
>  
> +static Property cpu_common_properties[] = {
> +DEFINE_PROP_INT32("nr-cores", CPUState, nr_cores, 1),
> +DEFINE_PROP_INT32("nr-threads", CPUState, nr_threads, 1),
> +DEFINE_PROP_END_OF_LIST()
> +};

Are you aware of the current CPU hotplug discussion that is going on?
I'm not very involved there, but I think some of these reworks also move
"nr_threads" into the CPU state already, e.g. see:

https://github.com/dgibson/qemu/commit/9d07719784ecbeebea71

... so you might want to check these patches first to see whether you
can base your rework on them?

 Thomas

Re: [Qemu-devel] [PATCH RFC 00/16] Rework SMP parameters

[Thomas Huth]

On 10.06.2016 19:40, Andrew Jones wrote:
> This series is a first step in eliminating smp_* global
> variables (the last patch gets rid of two of them!) And, it's
> a first step in deprecating '-smp' in favor of using machine
> properties, e.g.
>  qemu -machine pc,sockets=2,cores=2,threads=2,maxcpus=8,cpus=8 ...

Why isn't '-smp' good enough anymore so that it got to be deprecated?

 Thomas