Re: [Qemu-devel] [Qemu-block] [PATCH v4 00/12] fdc: fix 2.88mb floppy diskette support

[Roman Kagan]

On Wed, Jan 20, 2016 at 02:40:14PM -0500, John Snow wrote:
> On 01/20/2016 02:55 AM, Denis V. Lunev wrote:
> > should we recreate ACPI tables after geometry switch?
> > This would be especially interesting for the case of
> > Win2k12 (or Win8.1 if you prefer) under OVMF.
> > 
> > Den
> 
> This series doesn't really alter the concept that disk geometry can
> change at runtime -- Not knowing much about the ACPI reverse engineering
> that happened to make Windows 8/10 happy, does it work currently? Can
> you change to different density floppies and have it work out alright?

No, exactly because the geometry is determined once startup.

> If not, you can submit a patch against master as it is today -- this
> series only does two things:
> 
> (1) Alters the heuristics for which type of floppy drive is chosen at
> boot time (No change to ACPI table generation should be needed.)
> 
> (2) Allows 1.44MB diskettes to be recognized by 2.88MB drive types. This
> might require some changes, but check out pick_geometry both before and
> after this patchset -- there's a whole table of different geometries
> that we already allow users to switch between at runtime. If the
> geometry needs to update there, too, then it's already broken before
> this patchset.

Right.

This series conflicts slightly with the patches to generate ACPI objects
for floppies (which haven't made it into the mainstream qemu yet)
because of the adjustments in the floppy API.  Not a big deal.

> It should be easy enough to slide a geometry update in fd_revalidate()
> if needed.

Now that is a bit trickier: the currently submitted code queries the
floppy properties at SSDT build time, and sticks static objects into
AML; if that really needs updating at runtime it'll require certain
refactoring.

That said I'm not certain what exactly has to be done here.  Physical
machines do not have their floppy drives changable at runtime, do they?
So the OSes should be fine assuming that the drive stays the same, and
it's only the diskette that can change.  I'd guess that the OS driver
should do the necessary revalidation on its own, without ACPI
assistance; I'll give it a try when I have some time.

But again, as you said, people are mainly interested in floppies to
bootstrap a Windows installation on virtio disks, so support of floppy
geometry update at runtime is non-critical for most users.

Thanks,
Roman.

Re: [Qemu-devel] [PATCH v2 10/17] block: add generic full disk encryption driver

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 05:12:08PM +0800, Fam Zheng wrote:
> On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > +/* XXX Should we treat size as being total physical size
> > + * of the image (ie payload + encryption header), or just
> > + * the logical size of the image (ie payload). If the latter
> > + * then we need to extend 'size' to include the header
> > + * size */
> 
> The latter. :)

Ok

> > +qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, _abort);
> > +#define BLOCK_CRYPTO_DRIVER(name, format)   \
> > +static int block_crypto_probe_ ## name(const uint8_t *buf,  \
> > +   int buf_size,\
> > +   const char *filename) {  \
> > +return block_crypto_probe_generic(format,   \
> > +  buf, buf_size, filename); \
> > +}   \
> > +\
> > +static int block_crypto_open_ ## name(BlockDriverState *bs, \
> > +  QDict *options,   \
> > +  int flags,\
> > +  Error **errp) \
> > +{   \
> > +return block_crypto_open_generic(format,\
> > + _crypto_runtime_opts_ ## 
> > name, \
> > + bs, options, flags, errp); \
> > +}   \
> > +\
> > +static int block_crypto_create_ ## name(const char *filename,   \
> > +QemuOpts *opts, \
> > +Error **errp)   \
> > +{   \
> > +return block_crypto_create_generic(format,  \
> > +   filename, opts, errp);   \
> > +}   \
> > +\
> > +BlockDriver bdrv_crypto_ ## name = {\
> > +.format_name= #name,\
> > +.instance_size  = sizeof(BlockCrypto),  \
> > +.bdrv_probe = block_crypto_probe_ ## name,  \
> > +.bdrv_open  = block_crypto_open_ ## name,   \
> > +.bdrv_close = block_crypto_close,   \
> > +.bdrv_create= block_crypto_create_ ## name, \
> > +.create_opts= _crypto_create_opts_ ## name,   \
> > +\
> > +.bdrv_co_readv  = block_crypto_co_readv,\
> > +.bdrv_co_writev = block_crypto_co_writev,   \
> > +.bdrv_getlength = block_crypto_getlength,   \
> > +}
> > +
> > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS);
> 
> Personally I really prefer a preprocessed version, for the ease of grep.

I'm not sure I understand what you mean by a preprocessed version - could
you expand on that.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

[Qemu-devel] [PATCH v2 3/3] target-arm: Implement the S2 MMU inputsize > pamax check

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Implement the inputsize > pamax check for Stage 2 translations.
We have multiple choices for how to respond to errors and
choose to fault.

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/helper.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 4abeb4d..9a7ff5e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6808,7 +6808,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
  */
 int startlevel = extract32(tcr->raw_tcr, 6, 2);
 unsigned int pamax = arm_pamax(cpu);
-bool ok;
+bool ok = true;
 
 if (va_size == 32 || stride == 9) {
 /* AArch32 or 4KB pages */
@@ -6818,9 +6818,17 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
 level = 3 - startlevel;
 }
 
-/* Check that the starting level is valid. */
-ok = check_s2_startlevel(cpu, va_size == 64, level,
- inputsize, stride, pamax);
+if (va_size == 64 &&
+inputsize > pamax &&
+(arm_el_is_aa64(env, 1) || inputsize > 40)) {
+/* We have multiple choices but choose to fault.  */
+ok = false;
+}
+if (ok) {
+/* Check that the starting level is valid. */
+ok = check_s2_startlevel(cpu, va_size == 64, level,
+ inputsize, stride, pamax);
+}
 if (!ok) {
 /* AArch64 reports these as level 0 faults.
  * AArch32 reports these as level 1 faults.
-- 
1.9.1

[Qemu-devel] [PATCH v2 0/3] target-arm: Add a few more S2 MMU input checks

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This adds the inputsize > pamax check and also fixes the
startlevel checks to apply to the 64bit translations.

Comments welcome!

Cheers,
Edgar

v1 -> v2:
* inputsize > pmax check only applies to AArch64
* Fix commit message typo < should be >

Edgar E. Iglesias (3):
  target-arm: Apply S2 MMU startlevel table size check to AArch64
  target-arm: Make pamax an argument to check_s2_startlevel
  target-arm: Implement the S2 MMU inputsize > pamax check

 target-arm/helper.c | 38 +++---
 1 file changed, 23 insertions(+), 15 deletions(-)

-- 
1.9.1

[Qemu-devel] other cahpters about qemu

[Ata Fatahi baarzi]

hi all
i found a chapter named chapter  qemu detailed study hers
.
how can i find ther chapters?

-- 
Best Regrads
Ata Fatahi Baarzi 

Re: [Qemu-devel] live migration between different qemu versions

[Dr. David Alan Gilbert]

* Alexey (aluka...@alukardd.org) wrote:
> Hello,
> 
> On 2016-01-16 02:24, Eric Blake wrote:
> >On 01/12/2016 05:11 AM, Dr. David Alan Gilbert wrote:
> >
> >>>Tell me please right way to append zeros to "BIOS (ia32) ROM Ext.
> >>>(137*512)"
> >>>file?
> >>
> >>I'd use dd; something like:
> >>  dd if=/dev/zero bs=1 count=18944 >> theromfile
> >>I think that should do it (203264-184320=18944)
> >
> >Or simpler:
> >
> >truncate --size=203264 theromfile
> It's great! I have successfully did migration of VM from qemu 2.3.0 to qemu
> 2.5.0.
> 
> But when I try migrate to another side I got error:
> qemu-system-x86_64: error while loading state for instance 0x0 of device
> 'fw_cfg'
> qemu-system-x86_64: load of migration failed: No such file or directory
> 
> Is it possible to do this migration?

Backwards migration doens't get much upstream testing;  it looks like
this is the same as:

https://bugs.launchpad.net/qemu/+bug/1536487

Dave

> Regards,
> Alexey Mochkin.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v5 0/4] Extend TPM support with a QEMU-external TPM

[Stefan Berger]

"Dr. David Alan Gilbert"  wrote on 01/21/2016 
06:40:35 AM:

> 
> * Stefan Berger (stef...@us.ibm.com) wrote:
> > Stefan Berger/Watson/IBM wrote on 01/20/2016 02:51:58 PM:
> > 
> > > "Daniel P. Berrange"  wrote on 01/20/2016 
10:42:09 
> > AM:
> > > 
> > > > 
> > > > On Wed, Jan 20, 2016 at 10:23:50AM -0500, Stefan Berger wrote:
> > > > > "Daniel P. Berrange"  wrote on 01/20/2016 
> > 09:58:39 
> > > > > AM:
> > > > > 
> > > > > 
> > > > > > Subject: Re: [Qemu-devel] [PATCH v5 0/4] Extend TPM support 
with a 
> > 
> > > > > > QEMU-external TPM
> > > > > > 
> > > > > > On Mon, Jan 04, 2016 at 10:23:18AM -0500, Stefan Berger wrote:
> > > > > > > The following series of patches extends TPM support with an
> > > > > > > external TPM that offers a Linux CUSE (character device in 
> > userspace)
> > > > > > > interface. This TPM lets each VM access its own private 
vTPM.
> > > > > > 
> > > > > > What is the backing store for this vTPM ? Are the vTPMs all
> > > > > > multiplexed onto the host's physical TPM or is there something
> > > > > > else going on ?
> > > > > 
> > > > > The vTPM writes its state into a plain file. In case the user 
> > started the 
> > > > > vTPM, the user gets to choose the directory. In case of libvirt, 

> > libvirt 
> > > > > sets up the directory and starts the vTPM with the directory as 
a 
> > > > > parameter. The expectation for VMs (also containers) is that 
each VM 
> > can 
> > > > > use the full set of TPM commands with the vTPM and due to how 
the 
> > TPM 
> > > > > works, it cannot use the hardware TPM for that. SeaBIOS has 
> > beenextended 
> > > > > with TPM 1.2 support and initializes the vTPM in the same way it 

> > would 
> > > > > initialize a hardware TPM.
> > > > 
> > > > So if its using a plain file, then when snapshotting VMs we have 
to
> > > > do full copies of the file and keep them all around in sync with
> > > > the disk snapshots. By not having this functionality in QEMU we 
don't
> > > > immediately have a way to use qcow2 for the vTPM file backing 
store
> > > > to deal with snapshot management. The vTPM needs around 
snapshotting
> > > > feel fairly similar to the NVRAM needs, so it would be desiralbe 
to
> > > > have a ability to do a consistent thing for both.
> > > 
> > > The plain file serves as the current state of the TPM. In case of 
> > > migration, suspend, snapshotting, the vTPM state blobs are retrieved
> > > from the vTPM using ioctls and in case of a snapshot they are 
> > > written into the QCoW2. Upon resume the state blobs are set in the 
> > > vTPM. I is working as it is.
> > 
> > There is one issue in case of resume of a snapshot. If the permanent 
state 
> > of the TPM is modified during snapshotting, like ownership is taken of 
the 
> > TPM, the state, including the owner password, is written into the 
plain 
> > file. Then the VM is shut down. Once it is restarted (not a resume 
from a 
> > snapshot), the TPM's state will be relected by what was done during 
the 
> > run of that snapshot. So this is likely undesirable. Now the only way 
> > around this seems to be that one needs to know the reason for why the 
> > state blobs were pushed into the vTPM. In case of a snapshot, the 
writing 
> > of the permanent state into a file may need to be suppressed, while on 
a 
> > VM resume and resume from migration operation it needs to be written 
into 
> > the TPM's state file.
> 
> I don't understand that; are you saying that the ioctl's dont provide 
all
> the information that's included in the state file?

No. Running a snapshot does not change the state of the VM image unless 
one takes another snapshot. The vTPM has to be behave the same way, 
meaning that the state of the vTPM must not be overwritten while in a 
snapshot. However, the vTPM needs to know that it's running a snapshot 
whose state is 'volatile'.

Example: 
1) A VM is run and VM image is in state VM-A and vTPM is in state vTPM-A. 
The VM is shut down and VM is in state VM-A and vTPM is in state vTPM-A.

2) The VM runs a snapshot and VM image is in state VM-B and vTPM is in 
state B. The user takes ownership of the vTPM, which puts the vTPM into 
state vTPM-B2. VM is shut down and with that all VM image state is 
discarded. Also the VTPM's state needs to be discarded.

3) The VM is run again and the VM image is in state VM-A and the vTPM must 
be in state vTPM-A from 1). However, at the moment the vTPM wold be in 
state vTPM-B2 from the last run of the snapshot since the state was 
written into the vTPM's state file.

The way around the problem in 3) stemming from 2) is writing the vTPM 
state (which is kept in a file) into a differently named file while 
running a snapshot. However, QEMU needs to tell the vTPM that it's running 
a snapshot and the state is to be treated as volatile. A flag that conveys 
'you're running a snapshot' while setting the device state would be 
enough. Though currently the function 

[Qemu-devel] [PATCH] virt-acpi-build: add always-on property for timer

[Andrew Jones]

This patch is the ACPI equivalent of "hw/arm/virt: Add always-on
property to the virt board timer". The timer is always on, and
thus setting this informs Linux that it may switch off the periodic
timer. Switching off the periodic timer substantially reduces the
number of interrupts the host needs to inject.

Testing note: AArch64 guests (the only ones currently booting with
ACPI) do not actually need this patch to determine it can turn the
periodic timer off. I therefore used a hacked guest kernel to ensure
this patch works as the equivalent DT patch does.

Signed-off-by: Andrew Jones 
---
 hw/arm/virt-acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 87fbe7c97d99b..f6e538f3d02ea 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -443,7 +443,7 @@ build_gtdt(GArray *table_data, GArray *linker)
 gtdt->secure_el1_flags = ACPI_EDGE_SENSITIVE;
 
 gtdt->non_secure_el1_interrupt = ARCH_TIMER_NS_EL1_IRQ + 16;
-gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE;
+gtdt->non_secure_el1_flags = ACPI_EDGE_SENSITIVE | ACPI_GTDT_ALWAYS_ON;
 
 gtdt->virtual_timer_interrupt = ARCH_TIMER_VIRT_IRQ + 16;
 gtdt->virtual_timer_flags = ACPI_EDGE_SENSITIVE;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v9 2/3] quorum: implement bdrv_add_child() and bdrv_del_child()

[Alberto Garcia]

On Thu 21 Jan 2016 02:54:10 AM CET, Wen Congyang  wrote:

>>> @@ -875,9 +878,9 @@ static int quorum_open(BlockDriverState *bs, QDict 
>>> *options, int flags,
>>>  ret = -EINVAL;
>>>  goto exit;
>>>  }
>>> -if (s->num_children < 2) {
>>> +if (s->num_children < 1) {
>>>  error_setg(_err,
>>> -   "Number of provided children must be greater than 1");
>>> +   "Number of provided children must be 1 or more");
>>>  ret = -EINVAL;
>>>  goto exit;
>>>  }
>> 
>> I have a question: if you have a Quorum with just one member and you
>> add a new one, how do you know if it has the same data as the
>> existing one?
>> 
>> In general, what do you do to make sure that the data in a new Quorum
>> child is consistent with that of the rest of the array?
>
> Quorum can have more than one child when it starts. But we don't do
> the similar check. So I don't think we should do such check here.

Yes, but when you start a VM you can verify in advance that all members
of the Quorum have the same data. If you do that on a running VM how can
you know if the new disk is consistent with the others?

Berto

Re: [Qemu-devel] [PATCH v2 00/13] Dirty bitmap changes for migration/persistence work

[Fam Zheng]

On Thu, 01/21 13:41, Vladimir Sementsov-Ogievskiy wrote:
> An idea/question.
> 
> What about make some const dirty bitmap finctions really const?
> 
> bdrv_dirty_bitmap_name(BdrvDirtyBitmap *bitmap)   ->
> bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
> bdrv_dirty_bitmap_size(BdrvDirtyBitmap *bitmap) ->
> bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
> 
> etc..

That is fine. I think that is a good practice which is not always adopted.

Fam

Re: [Qemu-devel] [PATCH v2] xen-pvdevice: convert to realize()

[Cao jin]

Hi Stefano,

just want to make sure: is this picked up?

On 12/22/2015 07:51 PM, Stefano Stabellini wrote:

On Tue, 22 Dec 2015, Cao jin wrote:

Signed-off-by: Cao jin 


Acked-by: Stefano Stabellini 



  hw/i386/xen/xen_pvdevice.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)




--
Yours Sincerely,

Cao jin

[Qemu-devel] [PATCH 2/2] hmp: add hmp commands dirty bitmap add/clear/remove'

[Rudy Zhang]

Add several hmp commands: 'block_dirty_bitmap_add', 'block_dirty_bitmap_clear',
'block_dirty_bitmap_remove'. The bitmap is used for incremental backup to trace 
io.

Usage:
block_dirty_bitmap_add device bitmap [granularity] -- Add dirty bitmap 
for 'device'
block_dirty_bitmap_clear device bitmap -- Clear dirty bitmap for 
'device'
block_dirty_bitmap_remove device bitmap -- Remove dirty bitmap for 
'device'

Signed-off-by: Rudy Zhang 
---
 hmp-commands.hx | 42 +++
 hmp.c   | 68 +
 hmp.h   |  3 +++
 3 files changed, 113 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 7378aaa..8f1f95b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -154,6 +154,48 @@ ETEXI
 },
 
 STEXI
+@item block_dirty_bitmap_add
+@findex block_dirty_bitmap_add
+Add the block dirty bitmap.
+ETEXI
+
+{
+.name   = "block_dirty_bitmap_add",
+.args_type  = "node:B,bitmap:s,granularity:o?",
+.params = "device bitmap [granularity]",
+.help   = "Add dirty bitmap for 'device'",
+.mhandler.cmd = hmp_block_dirty_bitmap_add,
+},
+
+STEXI
+@item block_dirty_bitmap_clear
+@findex block_dirty_bitmap_clear
+Clear the block dirty bitmap.
+ETEXI
+
+{
+.name   = "block_dirty_bitmap_clear",
+.args_type  = "node:B,bitmap:s",
+.params = "device bitmap",
+.help   = "Clear dirty bitmap for 'device'",
+.mhandler.cmd = hmp_block_dirty_bitmap_clear,
+},
+
+STEXI
+@item block_dirty_bitmap_remove
+@findex block_dirty_bitmap_remove
+Remove the block dirty bitmap.
+ETEXI
+
+{
+.name   = "block_dirty_bitmap_remove",
+.args_type  = "node:B,bitmap:s",
+.params = "device bitmap",
+.help   = "Remove dirty bitmap for 'device'",
+.mhandler.cmd = hmp_block_dirty_bitmap_remove,
+},
+
+STEXI
 @item block_job_resume
 @findex block_job_resume
 Resume a paused block streaming operation.
diff --git a/hmp.c b/hmp.c
index f8c33cd..b5eecbc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1495,6 +1495,74 @@ void hmp_block_job_complete(Monitor *mon, const QDict 
*qdict)
 hmp_handle_error(mon, );
 }
 
+void hmp_block_dirty_bitmap_add(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *node = qdict_get_str(qdict, "node");
+const char *bitmap = qdict_get_str(qdict, "bitmap");
+uint32_t granularity = qdict_get_try_int(qdict, "granularity", 0);
+
+if (!node) {
+error_setg(, QERR_MISSING_PARAMETER, "node");
+hmp_handle_error(mon, );
+return;
+}
+
+if (!bitmap) {
+error_setg(, QERR_MISSING_PARAMETER, "bitmap");
+hmp_handle_error(mon, );
+return;
+}
+
+qmp_block_dirty_bitmap_add(node, bitmap,
+ !!granularity, granularity, );
+hmp_handle_error(mon, );
+}
+
+void hmp_block_dirty_bitmap_clear(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *node = qdict_get_str(qdict, "node");
+const char *bitmap = qdict_get_str(qdict, "bitmap");
+
+if (!node) {
+error_setg(, QERR_MISSING_PARAMETER, "node");
+hmp_handle_error(mon, );
+return;
+}
+
+if (!bitmap) {
+error_setg(, QERR_MISSING_PARAMETER, "bitmap");
+hmp_handle_error(mon, );
+return;
+}
+
+qmp_block_dirty_bitmap_clear(node, bitmap, );
+hmp_handle_error(mon, );
+}
+
+void hmp_block_dirty_bitmap_remove(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *node = qdict_get_str(qdict, "node");
+const char *bitmap = qdict_get_str(qdict, "bitmap");
+
+if (!node) {
+error_setg(, QERR_MISSING_PARAMETER, "node");
+hmp_handle_error(mon, );
+return;
+}
+
+if (!bitmap) {
+error_setg(, QERR_MISSING_PARAMETER, "bitmap");
+hmp_handle_error(mon, );
+return;
+}
+
+qmp_block_dirty_bitmap_remove(node, bitmap, );
+hmp_handle_error(mon, );
+}
+
 typedef struct HMPMigrationStatus
 {
 QEMUTimer *timer;
diff --git a/hmp.h b/hmp.h
index a8c5b5a..ea07116 100644
--- a/hmp.h
+++ b/hmp.h
@@ -81,6 +81,9 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_block_job_pause(Monitor *mon, const QDict *qdict);
 void hmp_block_job_resume(Monitor *mon, const QDict *qdict);
 void hmp_block_job_complete(Monitor *mon, const QDict *qdict);
+void hmp_block_dirty_bitmap_add(Monitor *mon, const QDict *qdict);
+void hmp_block_dirty_bitmap_clear(Monitor *mon, const QDict *qdict);
+void hmp_block_dirty_bitmap_remove(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_add(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
-- 
2.6.4

[Qemu-devel] [PATCH v2 3/5] kvm/x86: Pass return code of kvm_emulate_hypercall

[Andrey Smetanin]

Pass the return code from kvm_emulate_hypercall on to the caller,
in order to allow it to indicate to the userspace that
the hypercall has to be handled there.

Also adjust all the existing code paths to return 1 to make sure the
hypercall isn't passed to the userspace without setting kvm_run
appropriately.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c | 2 +-
 arch/x86/kvm/svm.c| 3 +--
 arch/x86/kvm/vmx.c| 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index f1a42e1..0e7c90f 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1055,7 +1055,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 */
if (kvm_x86_ops->get_cpl(vcpu) != 0 || !is_protmode(vcpu)) {
kvm_queue_exception(vcpu, UD_VECTOR);
-   return 0;
+   return 1;
}
 
longmode = is_64_bit_mode(vcpu);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c13a64b..9507038 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1858,8 +1858,7 @@ static int halt_interception(struct vcpu_svm *svm)
 static int vmmcall_interception(struct vcpu_svm *svm)
 {
svm->next_rip = kvm_rip_read(>vcpu) + 3;
-   kvm_emulate_hypercall(>vcpu);
-   return 1;
+   return kvm_emulate_hypercall(>vcpu);
 }
 
 static unsigned long nested_svm_get_tdp_cr3(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 04d61d4..82879aa 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5747,8 +5747,7 @@ static int handle_halt(struct kvm_vcpu *vcpu)
 
 static int handle_vmcall(struct kvm_vcpu *vcpu)
 {
-   kvm_emulate_hypercall(vcpu);
-   return 1;
+   return kvm_emulate_hypercall(vcpu);
 }
 
 static int handle_invd(struct kvm_vcpu *vcpu)
-- 
2.4.3

Re: [Qemu-devel] [PATCH RFC for-2.6 1/3] HBitmap: Introduce "meta" bitmap to track bit changes

[Vladimir Sementsov-Ogievskiy]

On 30.12.2015 14:07, Fam Zheng wrote:

On Wed, 12/30 13:53, Vladimir Sementsov-Ogievskiy wrote:

On 07.12.2015 08:59, Fam Zheng wrote:

The meta bitmap will have the same size and granularity as the tracked
bitmap, and upon each bit toggle, the corresponding bit in the meta
bitmap, at an identical position, will be set.

Signed-off-by: Fam Zheng 
---
  include/qemu/hbitmap.h |  7 +++
  util/hbitmap.c | 22 ++
  2 files changed, 29 insertions(+)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index bb94a00..09a6b06 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -181,6 +181,13 @@ void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap 
*hb, uint64_t first);
   */
  unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi);
+/* hbitmap_create_meta
+ * @hb: The HBitmap to operate on.
+ *
+ * Create a "meta" hbitmap to track dirtiness of the bits in this HBitmap.
+ */
+HBitmap *hbitmap_create_meta(HBitmap *hb);
+
  /**
   * hbitmap_iter_next:
   * @hbi: HBitmapIter to operate on.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 50b888f..3ad406e 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -81,6 +81,9 @@ struct HBitmap {
   */
  int granularity;
+/* A meta dirty bitmap to track the dirtiness of bits in this HBitmap. */
+HBitmap *meta;
+
  /* A number of progressively less coarse bitmaps (i.e. level 0 is the
   * coarsest).  Each bit in level N represents a word in level N+1 that
   * has a set bit, except the last level where each bit represents the
@@ -232,6 +235,7 @@ static inline bool hb_set_elem(unsigned long *elem, 
uint64_t start, uint64_t las
  /* The recursive workhorse (the depth is limited to HBITMAP_LEVELS)... */
  static void hb_set_between(HBitmap *hb, int level, uint64_t start, uint64_t 
last)
  {
+uint64_t save_start = start;
  size_t pos = start >> BITS_PER_LEVEL;
  size_t lastpos = last >> BITS_PER_LEVEL;
  bool changed = false;
@@ -252,6 +256,9 @@ static void hb_set_between(HBitmap *hb, int level, uint64_t 
start, uint64_t last
  }
  }
  changed |= hb_set_elem(>levels[level][i], start, last);
+if (hb->meta && level == HBITMAP_LEVELS - 1 && changed) {
+hbitmap_set(hb->meta, save_start, last - save_start + 1);
+}

I thing now, that the same may be accomplished for BdrvDirtyBitmap,
all we need is return "changed" status from hb_set_between and then
from hbitmap_set.

That is true, but it makes further optimizing to *really* only set the toggled
meta bits much more difficult (i.e. when only a few bits between start and last
are changed).


Are you going add some optimization in following versions (v3 ?) or as 
separate series, or other plan?




I haven't written any code for that optimization, but I did base my other
persistent dirty bitmap work on v2 of this series. It would be great if we can
harmonize on this, so we both have a common base of block dirty bitmap.

I can post v2 to the list very soon, if the idea is okay for you; or if you've
your preferred way, we can take a look together.  What do you think?

Fam


  /* If there was any change in this layer, we may have to update
   * the one above.
@@ -298,6 +305,7 @@ static inline bool hb_reset_elem(unsigned long *elem, 
uint64_t start, uint64_t l
  /* The recursive workhorse (the depth is limited to HBITMAP_LEVELS)... */
  static void hb_reset_between(HBitmap *hb, int level, uint64_t start, uint64_t 
last)
  {
+uint64_t save_start = start;
  size_t pos = start >> BITS_PER_LEVEL;
  size_t lastpos = last >> BITS_PER_LEVEL;
  bool changed = false;
@@ -336,6 +344,10 @@ static void hb_reset_between(HBitmap *hb, int level, 
uint64_t start, uint64_t la
  lastpos--;
  }
+if (hb->meta && level == HBITMAP_LEVELS - 1 && changed) {
+hbitmap_set(hb->meta, save_start, last - save_start + 1);
+}
+
  if (level > 0 && changed) {
  hb_reset_between(hb, level - 1, pos, lastpos);
  }
@@ -384,6 +396,9 @@ void hbitmap_free(HBitmap *hb)
  for (i = HBITMAP_LEVELS; i-- > 0; ) {
  g_free(hb->levels[i]);
  }
+if (hb->meta) {
+hbitmap_free(hb->meta);
+}
  g_free(hb);
  }
@@ -493,3 +508,10 @@ bool hbitmap_merge(HBitmap *a, const HBitmap *b)
  return true;
  }
+
+HBitmap *hbitmap_create_meta(HBitmap *hb)
+{
+assert(!hb->meta);
+hb->meta = hbitmap_alloc(hb->size, hb->granularity);
+return hb->meta;
+}


--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.




--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] qxl : no mouse cursor with sdl2/gtk UI

[Gerd Hoffmann]

On Do, 2016-01-21 at 11:38 +0100, nicolas prochazka wrote:
> it does not work , with dropping usb tablet.
> 
> to be more precise, when i write "no mouse cursor', it means, mouse is
> ok ( click and position) , it's just the mouse pointer which is not
> visible

Is the spice guest agent active?
Does it help to disable it?

cheers,
  Gerd

Re: [Qemu-devel] qxl : no mouse cursor with sdl2/gtk UI

[nicolas prochazka]

I test with and without spice guest agent,
nothing changes.

Regards,
Nicolas

2016-01-21 11:57 GMT+01:00 Gerd Hoffmann :

> On Do, 2016-01-21 at 11:38 +0100, nicolas prochazka wrote:
> > it does not work , with dropping usb tablet.
> >
> > to be more precise, when i write "no mouse cursor', it means, mouse is
> > ok ( click and position) , it's just the mouse pointer which is not
> > visible
>
> Is the spice guest agent active?
> Does it help to disable it?
>
> cheers,
>   Gerd
>
>
>

Re: [Qemu-devel] [PATCH v17 2/9] docs: vm generation id device's description

[Igor Mammedov]

On Wed, 20 Jan 2016 09:40:04 -0700
Eric Blake  wrote:

> On 01/19/2016 06:06 AM, Igor Mammedov wrote:
> > From: Gal Hammer 
> > 
> > Signed-off-by: Gal Hammer 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  docs/specs/vmgenid.txt | 36 
> >  1 file changed, 36 insertions(+)
> >  create mode 100644 docs/specs/vmgenid.txt
> > 
> > diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
> > new file mode 100644
> > index 000..718850a
> > --- /dev/null
> > +++ b/docs/specs/vmgenid.txt
> > @@ -0,0 +1,36 @@
> > +VIRTUAL MACHINE GENERATION ID
> > +=
> > +
> > +Copyright (C) 2014 Red Hat, Inc.
> 
> Do you want to add 2016?
Sure, I'll fix it up.

> 
> Otherwise,
> Reviewed-by: Eric Blake 
> 

Re: [Qemu-devel] [PATCH v9 19/37] qmp: Fix reference-counting of qnull on empty output visit

[Markus Armbruster]

Markus Armbruster  writes:

> Eric Blake  writes:
>
>> Commit 6c2f9a15 ensured that we would not return NULL when the
>> caller used an output visitor but had nothing to visit. But
>> in doing so, it added a FIXME about a reference count leak
>> that could abort qemu in the (unlikely) case of SIZE_MAX such
>> visits (more plausible on 32-bit).  (Although that commit
>> suggested we might fix it in time for 2.5, we ran out of time;
>> fortunately, it is unlikely enough to bite that it was not
>> worth worrying about during the 2.5 release.)
>>
>> This fixes things by documenting the internal contracts, and
>> explaining why the internal function can return NULL and only
>> the public facing interface needs to worry about qnull(),
>> thus avoiding over-referencing the qnull_ global object.
>>
>> It does not, however, fix the stupidity of the stack mixing
>> up two separate pieces of information; add a FIXME to explain
>> that issue, which will be fixed shortly in a future patch.
>>
>> Signed-off-by: Eric Blake 
>> Cc: qemu-sta...@nongnu.org
>> Reviewed-by: Marc-André Lureau 
>>
>> ---
>> v9: enhance commit message
>> v8: rebase to earlier changes
>> v7: cc qemu-stable, tweak some asserts, drop stale comment, add more
>> comments
>> v6: no change
>> ---
>>  qapi/qmp-output-visitor.c   | 39 ---
>>  tests/test-qmp-output-visitor.c |  2 ++
>>  2 files changed, 34 insertions(+), 7 deletions(-)
>>
>> diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
>> index d367148..316f4e4 100644
>> --- a/qapi/qmp-output-visitor.c
>> +++ b/qapi/qmp-output-visitor.c
[...]
>> @@ -41,10 +50,12 @@ static QmpOutputVisitor *to_qov(Visitor *v)
>>  return container_of(v, QmpOutputVisitor, visitor);
>>  }
>>
>> +/* Push @value onto the stack of current QObjects being built */
>>  static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
>>  {
>>  QStackEntry *e = g_malloc0(sizeof(*e));
>>
>> +assert(value);
>>  e->value = value;
>>  if (qobject_type(e->value) == QTYPE_QLIST) {
>>  e->is_list_head = true;
>> @@ -52,44 +63,51 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, 
>> QObject *value)
>>  QTAILQ_INSERT_HEAD(>stack, e, node);
>>  }
>>
>> +/* Grab and remove the most recent QObject from the stack */

Let's stick to the stack terminology you used for qmp_output_push_obj():

/* Pop a value off the stack of QObjects being built, and return it */

>>  static QObject *qmp_output_pop(QmpOutputVisitor *qov)
>>  {
>>  QStackEntry *e = QTAILQ_FIRST(>stack);
>>  QObject *value;
>> +
>> +assert(e);
>>  QTAILQ_REMOVE(>stack, e, node);
>>  value = e->value;
>>  g_free(e);
>>  return value;
>
> @value cannot be null here, because we never push nulls.  Worth an
> assertion, just to help readers?
>
>>  }
[...]
>> +/* Grab the most recent QObject from the stack, which must exist */

Stack terminology:

/*
 * Peek at the top of the stack of QObject being built.
 * The stack must not be empty.
 */

>>  static QObject *qmp_output_last(QmpOutputVisitor *qov)
>>  {
>>  QStackEntry *e = QTAILQ_FIRST(>stack);
>> +
>> +assert(e && e->value);
>>  return e->value;
>>  }
[...]

[Qemu-devel] [PATCH v2 1/5] kvm/x86: Rename Hyper-V long spin wait hypercall

[Andrey Smetanin]

Rename HV_X64_HV_NOTIFY_LONG_SPIN_WAIT
by HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT. So
the name better reflects hypercall codes accessory.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 2 +-
 arch/x86/kvm/hyperv.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 7956412..0c50fab 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -226,7 +226,7 @@
(~((1ull << HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT) - 1))
 
 /* Declare the various hypercall operations. */
-#define HV_X64_HV_NOTIFY_LONG_SPIN_WAIT0x0008
+#define HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT 0x0008
 
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ENABLE 0x0001
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT  12
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index c58ba67..f1a42e1 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1084,7 +1084,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
trace_kvm_hv_hypercall(code, fast, rep_cnt, rep_idx, ingpa, outgpa);
 
switch (code) {
-   case HV_X64_HV_NOTIFY_LONG_SPIN_WAIT:
+   case HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT:
kvm_vcpu_on_spin(vcpu);
break;
default:
-- 
2.4.3

[Qemu-devel] [PATCH v2 0/5] KVM: Hyper-V VMBus hypercalls

[Andrey Smetanin]

The patch implements userspace exit 'KVM_EXIT_HYPERV'
for Hyper-V VMBus hypercalls(postmsg, signalevent)
to handle these hypercalls by QEMU.

Changes v2:
* use KVM_EXIT_HYPERV for hypercalls

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

Andrey Smetanin (5):
  kvm/x86: Rename Hyper-V long spin wait hypercall
  drivers/hv: Move VMBus hypercall codes into Hyper-V UAPI header
  kvm/x86: Pass return code of kvm_emulate_hypercall
  kvm/x86: Reject Hyper-V hypercall continuation
  kvm/x86: Hyper-V VMBus hypercall userspace exit

 Documentation/virtual/kvm/api.txt  |  6 ++
 arch/x86/include/uapi/asm/hyperv.h |  4 +++-
 arch/x86/kvm/hyperv.c  | 40 +-
 arch/x86/kvm/hyperv.h  |  1 +
 arch/x86/kvm/svm.c |  3 +--
 arch/x86/kvm/vmx.c |  3 +--
 arch/x86/kvm/x86.c |  5 +
 drivers/hv/hv.c|  4 ++--
 drivers/hv/hyperv_vmbus.h  |  6 --
 include/uapi/linux/kvm.h   |  6 ++
 10 files changed, 56 insertions(+), 22 deletions(-)

-- 
2.4.3

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Laszlo Ersek]

On 01/21/16 12:42, Michael Tokarev wrote:
> 21.01.2016 14:37, Richard W.M. Jones wrote:
> 
>> I'm afraid I gave up on this -- did give it my best.  It turns out
>> that the machine that I thought supported UEFI boot does not.  I'll
>> keep an eye out for such a machine and test this in future.
> 
> BTW, why do you guys refer to UEFI boot all the time?  The whole thing
> is equally useful on traditional BIOS-booting machines too.  Windows7
> uses the same mechanism (looking at RSDT) for offline activation no
> matter if it is EFI system or not.
> 
> When I did first version of my patch (based on someone else's version,
> but details escapes my memory already), there was no UEFI support for
> qemu whatsoever, and it allowed me to use my OEM version of Windows7
> inside a qemu virtual machine without second activation.

I've been referring to UEFI because my interest was renewed in this
topic by the following OVMF problem report:

https://github.com/tianocore/edk2/issues/5

Of course, the issue is not specific to OVMF (that's why I posted the
patches for QEMU), but I guess it stuck in my mind. Sorry.

Laszlo

Re: [Qemu-devel] live migration between different qemu versions

[Alexey]

Hello.

On 2016-01-21 14:49, Dr. David Alan Gilbert wrote:

* Alexey (aluka...@alukardd.org) wrote:

Hello,

On 2016-01-16 02:24, Eric Blake wrote:
>On 01/12/2016 05:11 AM, Dr. David Alan Gilbert wrote:
>
>>>Tell me please right way to append zeros to "BIOS (ia32) ROM Ext.
>>>(137*512)"
>>>file?
>>
>>I'd use dd; something like:
>>  dd if=/dev/zero bs=1 count=18944 >> theromfile
>>I think that should do it (203264-184320=18944)
>
>Or simpler:
>
>truncate --size=203264 theromfile
It's great! I have successfully did migration of VM from qemu 2.3.0 to 
qemu

2.5.0.

But when I try migrate to another side I got error:
qemu-system-x86_64: error while loading state for instance 0x0 of 
device

'fw_cfg'
qemu-system-x86_64: load of migration failed: No such file or 
directory


Is it possible to do this migration?


Backwards migration doens't get much upstream testing;  it looks like
this is the same as:

https://bugs.launchpad.net/qemu/+bug/1536487

I saw it, thanks.



Dave


Regards,
Alexey Mochkin.

--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK


Regards,
Alexey Mochkin

[Qemu-devel] [PATCH] gtk: use qemu_chr_alloc() to allocate CharDriverState

[Daniel P. Berrange]

The gd_vc_handler() callback is using g_malloc0() to
allocate the CharDriverState struct. As a result the
logfd field is getting initialized to 0, instead of
-1 when no logfile is requested.

The result is that when running

 $ qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0

qemu duplicates all monitor output to stdout as well
as the GTK window.

Not using qemu_chr_alloc() was already a bug, but harmless
until this commit

  commit d0d7708ba29cbcc343364a46bff981e0ff88366f
  Author: Daniel P. Berrange 
  Date:   Mon Jan 11 12:44:41 2016 +

qemu-char: add logfile facility to all chardev backends

which exposed the problem as a behaviour regression

Reported-by: Hervé Poussineau 
Signed-off-by: Daniel P. Berrange 
---
 ui/gtk.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index ce7018e..c8dbd5c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1598,11 +1598,16 @@ static void gd_vc_chr_set_echo(CharDriverState *chr, 
bool echo)
 static int nb_vcs;
 static CharDriverState *vcs[MAX_VCS];
 
-static CharDriverState *gd_vc_handler(ChardevVC *unused, Error **errp)
+static CharDriverState *gd_vc_handler(ChardevVC *vc, Error **errp)
 {
+ChardevCommon *common = qapi_ChardevVC_base(vc);
 CharDriverState *chr;
 
-chr = g_malloc0(sizeof(*chr));
+chr = qemu_chr_alloc(common, errp);
+if (!chr) {
+return NULL;
+}
+
 chr->chr_write = gd_vc_chr_write;
 chr->chr_set_echo = gd_vc_chr_set_echo;
 
-- 
2.5.0

Re: [Qemu-devel] [PATCH v5 1/4] Provide support for the CUSE TPM

[Stefan Berger]

"Michael S. Tsirkin"  wrote on 01/21/2016 12:08:20 AM:


> 
> Well that was just one idea, it's up to you guys.
> But while modular multi-process QEMU for security
> might happen in future, I don't see us doing this
> by moving large parts of QEMU into cuse devices,
> and talking to these through ioctls.

I guess we'll have to rely one someone else to provide a different 
interface.

   Stefan
=

Re: [Qemu-devel] [PULL 0/2] VFIO updates 2016-01-19

[Peter Maydell]

On 19 January 2016 at 19:17, Alex Williamson  wrote:
> The following changes since commit 3db34bf64ab4f8797565dd8750003156c32b301d:
>
>   Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-peter' 
> into staging (2016-01-18 17:40:50 +)
>
> are available in the git repository at:
>
>
>   git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20160119.0
>
> for you to fetch changes up to 95239e162518dc6577164be3d9a789aba7f591a3:
>
>   vfio/pci: Lazy PBA emulation (2016-01-19 11:33:42 -0700)
>
> 
> VFIO updates 2016-01-19
>
>  - Performance fix for devices with poorly placed MSI-X PBA regions
>  - Quirk fix for hosts with broken MMCONFIG access
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v6 0/6] Xen PCI passthru: Convert to realize()

[Cao jin]

Hi Stefano,

Just FYI: The series have been reviewed by Eric, I guess it could 
be picked up;)


On 01/17/2016 08:13 PM, Cao jin wrote:

v6 changelog:
1. split modification of xen_host_pci_sysfs_path() into a separate new patch
as 1/6 shows.
2. 'bug' fix of qemu_strtoul(), in patch 2/6 & 3/6
3. Grammar fix in patch 4/6
4. 'msg' --> 'message' in commit message.

Cao jin (6):
   Change xen_host_pci_sysfs_path() to return void
   Xen: use qemu_strtoul instead of strtol
   Add Error **errp for xen_host_pci_device_get()
   Add Error **errp for xen_pt_setup_vga()
   Add Error **errp for xen_pt_config_init()
   Xen PCI passthru: convert to realize()

  hw/xen/xen-host-pci-device.c | 149 +--
  hw/xen/xen-host-pci-device.h |   5 +-
  hw/xen/xen_pt.c  |  77 --
  hw/xen/xen_pt.h  |   5 +-
  hw/xen/xen_pt_config_init.c  |  51 ---
  hw/xen/xen_pt_graphics.c |  11 ++--
  6 files changed, 155 insertions(+), 143 deletions(-)



--
Yours Sincerely,

Cao jin

[Qemu-devel] [PATCH v2 4/5] kvm/x86: Reject Hyper-V hypercall continuation

[Andrey Smetanin]

Currently we do not support Hyper-V hypercall continuation
so reject it.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 0e7c90f..e1daa8b 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1083,6 +1083,12 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 
trace_kvm_hv_hypercall(code, fast, rep_cnt, rep_idx, ingpa, outgpa);
 
+   /* Hypercall continuation is not supported yet */
+   if (rep_cnt || rep_idx) {
+   res = HV_STATUS_INVALID_HYPERCALL_CODE;
+   goto set_result;
+   }
+
switch (code) {
case HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT:
kvm_vcpu_on_spin(vcpu);
@@ -1092,6 +1098,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
break;
}
 
+set_result:
ret = res | (((u64)rep_done & 0xfff) << 32);
if (longmode) {
kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
-- 
2.4.3

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Paolo Bonzini]

On 21/01/2016 11:39, Gerd Hoffmann wrote:
> How does BH signaling work?  I know I can qemu_bh_schedule from !main
> thread to kick BH in main thread context.  The other way around works
> using aio_bh_new + aio_bh_call I guess?

Yes.  The code will run in the iothread.  Note that I'm modifying
AioContext to *require* an IOThread if you use it outside the main thread.

Paolo

Re: [Qemu-devel] [PATCH v2 04/17] crypto: add support for generating initialization vectors

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 03:51:37PM +0800, Fam Zheng wrote:
> On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > +static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen,
> > +const uint8_t *key, size_t nkey,
> > +Error **errp)
> > +{
> > +uint8_t *salt;
> > +size_t nhash;
> > +QCryptoIVGenESSIV *essiv = g_new0(QCryptoIVGenESSIV, 1);
> > +
> > +nhash = qcrypto_hash_digest_len(ivgen->hash);
> > +/* Salt must be larger of hash size or key size */
> > +salt = g_new0(uint8_t, nhash > nkey ? nhash : nkey);
> > +
> > +if (qcrypto_hash_bytes(ivgen->hash, (const gchar *)key, nkey,
> > +   , ,
> > +   errp) < 0) {
> > +g_free(essiv);
> > +return -1;
> > +}
> > +
> > +essiv->cipher = qcrypto_cipher_new(ivgen->cipher,
> > +   QCRYPTO_CIPHER_MODE_ECB,
> > +   salt, nkey,
> > +   errp);
> > +if (!essiv->cipher) {
> > +g_free(essiv);
> > +g_free(salt);
> > +return -1;
> > +}
> > +
> > +g_free(salt);
> > +ivgen->private = essiv;
> > +
> > +return 0;
> > +}
> > +
> > +static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
> > + uint64_t sector,
> > + uint8_t *iv, size_t niv,
> > + Error **errp)
> > +{
> > +QCryptoIVGenESSIV *essiv = ivgen->private;
> > +size_t ndata = qcrypto_cipher_get_block_len(ivgen->cipher);
> > +uint8_t *data = g_new(uint8_t, ndata);
> > +
> > +sector = cpu_to_le64((uint32_t)sector);
> 
> Why casting to 32 bit?

Bugtastic.

> > +memcpy(data, (uint8_t *), ndata);
> > +if (sizeof(sector) < ndata) {
> > +memset(data + sizeof(sector), 0, ndata - sizeof(sector));
> > +}
> > +
> > +if (qcrypto_cipher_encrypt(essiv->cipher,
> > +   data,
> > +   data,
> > +   ndata,
> > +   errp) < 0) {
> > +g_free(data);
> > +return -1;
> > +}
> > +
> > +if (ndata > niv) {
> > +ndata = niv;
> > +}
> > +memcpy(iv, data, ndata);
> > +if (ndata < niv) {
> > +memset(iv + ndata, 0, niv - ndata);
> > +}
> > +g_free(data);
> > +return 0;
> > +}
> > +
> > +static void qcrypto_ivgen_essiv_cleanup(QCryptoIVGen *ivgen)
> > +{
> > +QCryptoIVGenESSIV *essiv = ivgen->private;
> > +
> > +qcrypto_cipher_free(essiv->cipher);
> > +g_free(essiv);
> > +}
> > +
> > +
> > +struct QCryptoIVGenDriver qcrypto_ivgen_essiv = {
> > +qcrypto_ivgen_essiv_init,
> 
> It'd be more readable to explicitly write
> 
> .init = qcrypto_ivgen_essiv_init,
> .calculate = ...,
> .cleanup = ...
> 
> but it is fine since there are only three operations now.

Yeah, I normally do it that way anyway, so will change it.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 05/17] crypto: add support for anti-forensic split algorithm

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 04:37:28PM +0800, Fam Zheng wrote:
> 
> > +
> > +/**
> > + * qcrypto_afsplit_encode:
> > + * @hash: the hash algorithm to use for data expansion
> > + * @blocklen: the size of @in in bytes
> > + * @stripes: the number of times to expand @in in size
> > + * @in: the master key to be expanded in size
> > + * @out: preallocted buffer to hold the split key
> > + * @errp: pointer to a NULL-initialized error object
> > + *
> > + * Split the data in @in, which is @blocklen bytes in
> > + * size, to form a larger piece of data @out, which is
> > + * @blocklen * @stripes bytes in size.
> > + *
> > + * Returns: 0 on success, -1 on error;
> > + */
> > +int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
> > +   size_t blocklen,
> > +   uint32_t stripes,
> > +   const uint8_t *in,
> > +   uint8_t *out,
> > +   Error **errp);
> > +
> > +/**
> > + * qcrypto_afsplit_decode:
> > + * @hash: the hash algorithm to use for data compression
> > + * @blocklen: the size of @out in bytes
> > + * @stripes: the number of times to decrease @in in size
> > + * @in: the master key to be expanded in size
> > + * @out: preallocted buffer to hold the split key
> 
> I think the descriptions for @in and @out are wrong.

Yeah, got them the wrong way around

> 
> > + * @errp: pointer to a NULL-initialized error object
> > + *
> > + * Join the data in @in, which is @blocklen * @stripes
> > + * bytes in size, to form the original small piece o
> 
> piece of
> 
> > + * data @out, which is @blocklen bytes in size.
> > + *
> > + * Returns: 0 on success, -1 on error;
> > + */
> > +int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
> > +   size_t blocklen,
> > +   uint32_t stripes,
> > +   const uint8_t *in,
> > +   uint8_t *out,
> > +   Error **errp);
> > +
> > +#endif /* QCRYPTO_AFSPLIT_H__ */
> 
> Fam

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 10/17] block: add generic full disk encryption driver

[Fam Zheng]

On Thu, 01/21 11:02, Daniel P. Berrange wrote:
> On Thu, Jan 21, 2016 at 05:12:08PM +0800, Fam Zheng wrote:
> > On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > > +/* XXX Should we treat size as being total physical size
> > > + * of the image (ie payload + encryption header), or just
> > > + * the logical size of the image (ie payload). If the latter
> > > + * then we need to extend 'size' to include the header
> > > + * size */
> > 
> > The latter. :)
> 
> Ok
> 
> > > +qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, _abort);
> > > +#define BLOCK_CRYPTO_DRIVER(name, format)   \
> > > +static int block_crypto_probe_ ## name(const uint8_t *buf,  \
> > > +   int buf_size,\
> > > +   const char *filename) {  \
> > > +return block_crypto_probe_generic(format,   \
> > > +  buf, buf_size, filename); \
> > > +}   \
> > > +\
> > > +static int block_crypto_open_ ## name(BlockDriverState *bs, \
> > > +  QDict *options,   \
> > > +  int flags,\
> > > +  Error **errp) \
> > > +{   \
> > > +return block_crypto_open_generic(format,\
> > > + _crypto_runtime_opts_ ## 
> > > name, \
> > > + bs, options, flags, errp); \
> > > +}   \
> > > +\
> > > +static int block_crypto_create_ ## name(const char *filename,   \
> > > +QemuOpts *opts, \
> > > +Error **errp)   \
> > > +{   \
> > > +return block_crypto_create_generic(format,  \
> > > +   filename, opts, errp);   \
> > > +}   \
> > > +\
> > > +BlockDriver bdrv_crypto_ ## name = {\
> > > +.format_name= #name,\
> > > +.instance_size  = sizeof(BlockCrypto),  \
> > > +.bdrv_probe = block_crypto_probe_ ## name,  \
> > > +.bdrv_open  = block_crypto_open_ ## name,   \
> > > +.bdrv_close = block_crypto_close,   \
> > > +.bdrv_create= block_crypto_create_ ## name, \
> > > +.create_opts= _crypto_create_opts_ ## name,   \
> > > +\
> > > +.bdrv_co_readv  = block_crypto_co_readv,\
> > > +.bdrv_co_writev = block_crypto_co_writev,   \
> > > +.bdrv_getlength = block_crypto_getlength,   \
> > > +}
> > > +
> > > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS);
> > 
> > Personally I really prefer a preprocessed version, for the ease of grep.
> 
> I'm not sure I understand what you mean by a preprocessed version - could
> you expand on that.

I mean don't use macro concatenation and use plain symbols like in other block
drivers.

BlockDriver bdrv_crypto_luks = {
.format_name= "luks",
.instance_size  = sizeof(BlockCrypto),
.bdrv_probe = block_crypto_probe_luks,
.bdrv_open  = block_crypto_open_luks,
...

mostly because it's easier to grep (or for refactoring with tools).

But I can't how repeatitive this would be (I can see the "don't repeat
yourself" with your approach).  There is only one BLOCK_CRYPTO_DRIVER instance
in this series. This is probably bikeshedding.

Fam

Re: [Qemu-devel] [PATCH v2 15/17] block: rip out all traces of password prompting

[Fam Zheng]

On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
> index 3386668..065d9af 100755
> --- a/tests/qemu-iotests/087
> +++ b/tests/qemu-iotests/087
> @@ -201,6 +201,7 @@ run_qemu -S <"options": {
>  "driver": "$IMGFMT",
>  "id": "disk",
> +"key-secret": "sec0",
>  "file": {
>  "driver": "file",
>  "filename": "$TEST_IMG"
> @@ -228,6 +229,7 @@ run_qemu <"options": {
>  "driver": "$IMGFMT",
>  "id": "disk",
> +"key-secret": "sec0",
>  "file": {
>  "driver": "file",
>  "filename": "$TEST_IMG"
> diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> index 5fc4823..6582dda 100644
> --- a/tests/qemu-iotests/087.out
> +++ b/tests/qemu-iotests/087.out
> @@ -49,7 +49,7 @@ QMP_VERSION
>  Encrypted images are deprecated
>  Support for them will be removed in a future release.
>  You can use 'qemu-img convert' to convert your image to an unencrypted one.
> -{"error": {"class": "GenericError", "desc": "blockdev-add doesn't support 
> encrypted devices"}}
> +{"return": {}}
>  {"return": {}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN"}
>  
> @@ -60,7 +60,7 @@ QMP_VERSION
>  Encrypted images are deprecated
>  Support for them will be removed in a future release.
>  You can use 'qemu-img convert' to convert your image to an unencrypted one.
> -{"error": {"class": "GenericError", "desc": "Guest must be stopped for 
> opening of encrypted image"}}
> +{"return": {}}
>  {"return": {}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN"}
>  
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index d9913f8..da78459 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -135,9 +135,9 @@ _make_test_img()
>  # XXX(hch): have global image options?
>  (
>   if [ $use_backing = 1 ]; then
> -$QEMU_IMG create -f $IMGFMT $extra_img_options -b "$backing_file" 
> "$img_name" $image_size 2>&1
> +$QEMU_IMG create $EXTRA_IMG_ARGS -f $IMGFMT $extra_img_options -b 
> "$backing_file" "$img_name" $image_size 2>&1
>   else
> -$QEMU_IMG create -f $IMGFMT $extra_img_options "$img_name" 
> $image_size 2>&1
> +$QEMU_IMG create $EXTRA_IMG_ARGS -f $IMGFMT $extra_img_options 
> "$img_name" $image_size 2>&1
>   fi
>  ) | _filter_img_create
>  

Does this change belong to a separate patch?

Fam

Re: [Qemu-devel] [PATCH v9 20/37] qmp: Don't abuse stack to track qmp-output root

[Markus Armbruster]

Eric Blake  writes:

> The previous commit documented an inconsistency in how we are
> using the stack of qmp-output-visitor.  Normally, pushing a
> single top-level object puts the object on the stack twice:
> once as the root, and once as the current container being
> appended to; but popping that struct only pops once.  However,
> qmp_ouput_add() was trying to either set up the added object
> as the new root (works if you parse two top-level scalars in a
> row: the second replaces the first as the root) or as a member
> of the current container (works as long as you have an open
> container on the stack; but if you have popped the first
> top-level container, it then resolves to the root and still
> tries to add into that existing container).
>
> Fix the stupidity by not tracking two separate things in the
> stack.  Drop the now-useless qmp_output_first() while at it.
>
> Saved for a later patch: we still are rather sloppy in that
> qmp_output_get_object() can be called in the middle of a parse,
> rather than requiring that a visit is complete.
>
> Signed-off-by: Eric Blake 
> Reviewed-by: Marc-André Lureau 
>
> ---
> v9: no change
> v8: rebase to earlier changes
> v7: retitle; rebase to earlier changes, drop qmp_output_first()
> v6: no change
> ---
>  qapi/qmp-output-visitor.c | 79 
> ---
>  1 file changed, 26 insertions(+), 53 deletions(-)
>
> diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
> index 316f4e4..df22999 100644
> --- a/qapi/qmp-output-visitor.c
> +++ b/qapi/qmp-output-visitor.c
> @@ -29,16 +29,8 @@ typedef QTAILQ_HEAD(QStack, QStackEntry) QStack;
>  struct QmpOutputVisitor
>  {
>  Visitor visitor;
> -/* FIXME: we are abusing stack to hold two separate pieces of
> - * information: the current root object in slot 0, and the stack
> - * of N objects still being built in slots 1 through N (for N+1
> - * slots in use).  Worse, our behavior is inconsistent:
> - * qmp_output_add_obj() visiting two top-level scalars in a row
> - * discards the first in favor of the second, but visiting two
> - * top-level objects in a row tries to append the second object
> - * into the first (since the first object was placed in the stack
> - * in both slot 0 and 1, but only popped from slot 1).  */
> -QStack stack;
> +QStack stack; /* Stack of containers that haven't yet been finished */
> +QObject *root; /* Root of the output visit */
>  };
>
>  #define qmp_output_add(qov, name, value) \
> @@ -55,6 +47,7 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, 
> QObject *value)
>  {
>  QStackEntry *e = g_malloc0(sizeof(*e));
>
> +assert(qov->root);

This is safe because every call is preceded by qmp_output_add(), which
creates the root if we don't have one, yet.

>  assert(value);
>  e->value = value;
>  if (qobject_type(e->value) == QTYPE_QLIST) {
> @@ -76,26 +69,12 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
>  return value;
>  }
>
> -/* Grab the root QObject, if any, in preparation to empty the stack */
> -static QObject *qmp_output_first(QmpOutputVisitor *qov)
> -{
> -QStackEntry *e = QTAILQ_LAST(>stack, QStack);
> -
> -if (!e) {
> -/* No root */
> -return NULL;
> -}
> -assert(e->value);
> -return e->value;
> -}
> -
> -/* Grab the most recent QObject from the stack, which must exist */
> +/* Grab the most recent QObject from the stack, if any */
>  static QObject *qmp_output_last(QmpOutputVisitor *qov)
>  {
>  QStackEntry *e = QTAILQ_FIRST(>stack);
>
> -assert(e && e->value);
> -return e->value;
> +return e ? e->value : NULL;
>  }

Okay, because...

>
>  /* Add @value to the current QObject being built.
> @@ -106,29 +85,25 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov, 
> const char *name,
>  {
>  QObject *cur;
>
> -if (QTAILQ_EMPTY(>stack)) {
> -/* Stack was empty, track this object as root */
> -qmp_output_push_obj(qov, value);
> -return;
> -}
> -
>  cur = qmp_output_last(qov);

... this is the only caller, and...

>
> -switch (qobject_type(cur)) {
> -case QTYPE_QDICT:
> -assert(name);
> -qdict_put_obj(qobject_to_qdict(cur), name, value);
> -break;
> -case QTYPE_QLIST:
> -qlist_append_obj(qobject_to_qlist(cur), value);
> -break;
> -default:
> -/* The previous root was a scalar, replace it with a new root */
> -/* FIXME this is abusing the stack; see comment above */
> -qobject_decref(qmp_output_pop(qov));
> -assert(QTAILQ_EMPTY(>stack));
> -qmp_output_push_obj(qov, value);
> -break;
> +if (!cur) {

... you add the null check.

> +/* FIXME we should require the user to reset the visitor, rather
> + * than throwing away the previous root */
> +

Re: [Qemu-devel] [PATCH v5 1/4] Provide support for the CUSE TPM

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berra...@redhat.com) wrote:
> On Wed, Jan 20, 2016 at 10:54:47AM -0500, Stefan Berger wrote:
> > On 01/20/2016 10:46 AM, Daniel P. Berrange wrote:
> > >On Wed, Jan 20, 2016 at 10:31:56AM -0500, Stefan Berger wrote:
> > >>"Daniel P. Berrange"  wrote on 01/20/2016 10:00:41
> > >>AM:
> > >>
> > >>
> > >>>process at all - it would make sense if there was a single
> > >>>swtpm_cuse shared across all QEMU's, but if there's one per
> > >>>QEMU device, it feels like it'd be much simpler to just have
> > >>>the functionality linked in QEMU.  That avoids the problem
> > >>I tried having it linked in QEMU before. It was basically rejected.
> > >I remember an impl you did many years(?) ago now, but don't recall
> > >the results of the discussion. Can you elaborate on why it was
> > >rejected as an approach ? It just doesn't make much sense to me
> > >to have to create an external daemon, a CUSE device and comms
> > >protocol, simply to be able to read/write a plain file containing
> > >the TPM state. Its massive over engineering IMHO and adding way
> > >more complexity and thus scope for failure
> > 
> > The TPM 1.2 implementation adds 10s of thousands of lines of code. The TPM 2
> > implementation is in the same range. The concern was having this code right
> > in the QEMU address space. It's big, it can have bugs, so we don't want it
> > to harm QEMU. So we now put this into an external process implemented by the
> > swtpm project that builds on libtpms which provides TPM 1.2 functionality
> > (to be extended with TPM 2). We cannot call APIs of libtpms directly
> > anymore, so we need a control channel, which is implemented through ioctls
> > on the CUSE device.
> 
> Ok, the security separation concern does make some sense. The use of CUSE
> still seems fairly questionable to me. CUSE makes sense if you want to
> provide a drop-in replacement for the kernel TPM device driver, which
> would avoid ned for a new QEMU backend. If you're not emulating an existing
> kernel driver ABI though, CUSE + ioctl is feels like a really awful RPC
> transport between 2 userspace processes.

While I don't really like CUSE; I can see some of the reasoning here.
By providing the existing TPM ioctl interface I think it means you can use
existing host-side TPM tools to initialise/query the soft-tpm, and those
should be independent of the soft-tpm implementation.
As for the extra interfaces you need because it's a soft-tpm to set it up,
once you've already got that ioctl interface as above, then it seems to make
sense to extend that to add the extra interfaces needed.  The only thing
you have to watch for there are that the extra interfaces don't clash
with any future kernel ioctl extensions, and that the interface defined
is generic enough for different soft-tpm implementations.

Dave


> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o- http://virt-manager.org :|
> |: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Richard W.M. Jones]

On Thu, Jan 14, 2016 at 05:35:21PM +0100, Laszlo Ersek wrote:
> On 01/14/16 11:23, Richard W.M. Jones wrote:
> > On Thu, Jan 14, 2016 at 01:06:05PM +0300, Alex wrote:
> >> Richard, I just posted HW test results to
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1248758.
> >> Should I do it here instead?
> > 
> > I saw that.  Testing a virt-p2v conversion is a lot more involved.  It
> > would involve something like this:
> > 
> > (1) Install Win7 on a UEFI-based physical machine, ensuring that Win7
> > is using UEFI to boot (not CSM or BIOS).
> > 
> > (2) Install a recent Fedora on a second machine (second machine may be
> > a VM).  'dnf install virt-v2v' on this machine.
> > 
> > (3) Boot virt-p2v ISO (http://oirase.annexia.org/virt-p2v/) on the
> > first physical machine.  Perform a P2V conversion
> > (http://libguestfs.org/virt-p2v.1.html).
> > 
> > (4) Boot the converted Win7 VM on the target qemu.  Reproduce the
> > original bug.  We have never had the original bug reported to us by
> > any customer.
> > 
> > (5) Patch qemu on the target.
> > 
> > (6) Boot virt-p2v ISO again, and perform a second conversion.
> > 
> > (7) Verify that the bug (step 4) has been fixed.
> 
> Very good description, thank you.
> 
> > While I was looking at Laszlo's patches just now, I realized that I
> > have an AMD box that uses UEFI (actually - it uses CSM right now, but
> > I think I can make it boot using pure UEFI).  I'll have to swap some
> > disks around but I may be able to try this out today or tomorrow if I
> > can find a spare hard disk.
> 
> That would be awesome, yes. Thanks!

I'm afraid I gave up on this -- did give it my best.  It turns out
that the machine that I thought supported UEFI boot does not.  I'll
keep an eye out for such a machine and test this in future.

All was not lost because I did discover a few bugs in virt-p2v along
the way:

https://github.com/libguestfs/libguestfs/commit/7e2f2b0b2410587b81fd42bf741e3a36a5e75f6f
https://github.com/libguestfs/libguestfs/commit/c3ebc0a83761c552e6c19163b6d87044ae1ca635
https://github.com/libguestfs/libguestfs/commit/3f376fa5137d73df681cc5eaaa9b5e4206d57fce
https://github.com/libguestfs/libguestfs/commit/d723b352f8c10fb5244f17889cf68e13dd85c037

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [Qemu-devel] [PATCH 1/1] vl: change QEMU state machine for system reset

[Denis V. Lunev]

On 01/21/2016 01:35 PM, Paolo Bonzini wrote:


On 19/01/2016 08:59, Denis V. Lunev wrote:

@@ -612,8 +617,10 @@ static const RunStateTransition runstate_transitions_def[] 
= {
  
  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },

  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
+{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
  
  { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },

+{ RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
  
  { RUN_STATE_RUNNING, RUN_STATE_DEBUG },

  { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
@@ -627,20 +634,25 @@ static const RunStateTransition 
runstate_transitions_def[] = {
  { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
  
  { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },

+{ RUN_STATE_SAVE_VM, RUN_STATE_PRELAUNCH },

This should not happen; thus I would remove this line.


  { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
  { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH },
  
  { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },

  { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
  { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
  { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_SUSPENDED, RUN_STATE_PRELAUNCH },
  
  { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },

  { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_WATCHDOG, RUN_STATE_PRELAUNCH },
  
  { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },

  { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_GUEST_PANICKED, RUN_STATE_PRELAUNCH },
  
  { RUN_STATE__MAX, RUN_STATE__MAX },

  };
@@ -1886,8 +1899,10 @@ static bool main_loop_should_exit(void)
  cpu_synchronize_all_states();
  qemu_system_reset(VMRESET_REPORT);
  resume_all_vcpus();
-if (runstate_needs_reset()) {
-runstate_set(RUN_STATE_PAUSED);
+if (!runstate_check(RUN_STATE_RUNNING) &&
+!runstate_check(RUN_STATE_INMIGRATE) &&
+!runstate_check(RUN_STATE_SAVE_VM)) {

Since SAVE_VM should not happen here, I would leave this check out too.
  If it happens, the lack of a SAVE_VM->PRELAUNCH transition will cause
an assertion failure.

Thanks for the patch!

Paolo

:) cool. This one is tied to one of our internal bugs thus I have to 
finish it some way.

The rest will be handled in libvirt by my colleague.

I much more worry about bdrv_invalidate_cache(). This problem crashes QEMU
with 100% probability in our stress testing. Manually it crashes 1 times 
out of 5

('virsh managed-save && virsh start' in parallel with
'while /bin/true; do virsh qemu-monitor-command --hmp info block ; done'

Den

Re: [Qemu-devel] [PATCH v2 15/17] block: rip out all traces of password prompting

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 09:02:14PM +0800, Fam Zheng wrote:
> On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
> > index 3386668..065d9af 100755
> > --- a/tests/qemu-iotests/087
> > +++ b/tests/qemu-iotests/087
> > @@ -201,6 +201,7 @@ run_qemu -S < >"options": {
> >  "driver": "$IMGFMT",
> >  "id": "disk",
> > +"key-secret": "sec0",
> >  "file": {
> >  "driver": "file",
> >  "filename": "$TEST_IMG"
> > @@ -228,6 +229,7 @@ run_qemu < >"options": {
> >  "driver": "$IMGFMT",
> >  "id": "disk",
> > +"key-secret": "sec0",
> >  "file": {
> >  "driver": "file",
> >  "filename": "$TEST_IMG"
> > diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> > index 5fc4823..6582dda 100644
> > --- a/tests/qemu-iotests/087.out
> > +++ b/tests/qemu-iotests/087.out
> > @@ -49,7 +49,7 @@ QMP_VERSION
> >  Encrypted images are deprecated
> >  Support for them will be removed in a future release.
> >  You can use 'qemu-img convert' to convert your image to an unencrypted one.
> > -{"error": {"class": "GenericError", "desc": "blockdev-add doesn't support 
> > encrypted devices"}}
> > +{"return": {}}
> >  {"return": {}}
> >  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, 
> > "event": "SHUTDOWN"}
> >  
> > @@ -60,7 +60,7 @@ QMP_VERSION
> >  Encrypted images are deprecated
> >  Support for them will be removed in a future release.
> >  You can use 'qemu-img convert' to convert your image to an unencrypted one.
> > -{"error": {"class": "GenericError", "desc": "Guest must be stopped for 
> > opening of encrypted image"}}
> > +{"return": {}}
> >  {"return": {}}
> >  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, 
> > "event": "SHUTDOWN"}
> >  
> > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> > index d9913f8..da78459 100644
> > --- a/tests/qemu-iotests/common.rc
> > +++ b/tests/qemu-iotests/common.rc
> > @@ -135,9 +135,9 @@ _make_test_img()
> >  # XXX(hch): have global image options?
> >  (
> >   if [ $use_backing = 1 ]; then
> > -$QEMU_IMG create -f $IMGFMT $extra_img_options -b "$backing_file" 
> > "$img_name" $image_size 2>&1
> > +$QEMU_IMG create $EXTRA_IMG_ARGS -f $IMGFMT $extra_img_options -b 
> > "$backing_file" "$img_name" $image_size 2>&1
> >   else
> > -$QEMU_IMG create -f $IMGFMT $extra_img_options "$img_name" 
> > $image_size 2>&1
> > +$QEMU_IMG create $EXTRA_IMG_ARGS -f $IMGFMT $extra_img_options 
> > "$img_name" $image_size 2>&1
> >   fi
> >  ) | _filter_img_create
> >  
> 
> Does this change belong to a separate patch?

Opps, no. This change to common.rc should be deleted - it was something
I added which turned out to not be needed

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 10/17] block: add generic full disk encryption driver

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 09:01:19PM +0800, Fam Zheng wrote:
> On Thu, 01/21 11:02, Daniel P. Berrange wrote:
> > On Thu, Jan 21, 2016 at 05:12:08PM +0800, Fam Zheng wrote:
> > > On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > > > +/* XXX Should we treat size as being total physical size
> > > > + * of the image (ie payload + encryption header), or just
> > > > + * the logical size of the image (ie payload). If the latter
> > > > + * then we need to extend 'size' to include the header
> > > > + * size */
> > > 
> > > The latter. :)
> > 
> > Ok
> > 
> > > > +qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, _abort);
> > > > +#define BLOCK_CRYPTO_DRIVER(name, format)  
> > > >  \
> > > > +static int block_crypto_probe_ ## name(const uint8_t *buf, 
> > > >  \
> > > > +   int buf_size,   
> > > >  \
> > > > +   const char *filename) { 
> > > >  \
> > > > +return block_crypto_probe_generic(format,  
> > > >  \
> > > > +  buf, buf_size, filename);
> > > >  \
> > > > +}  
> > > >  \
> > > > +   
> > > >  \
> > > > +static int block_crypto_open_ ## name(BlockDriverState *bs,
> > > >  \
> > > > +  QDict *options,  
> > > >  \
> > > > +  int flags,   
> > > >  \
> > > > +  Error **errp)
> > > >  \
> > > > +{  
> > > >  \
> > > > +return block_crypto_open_generic(format,   
> > > >  \
> > > > + _crypto_runtime_opts_ 
> > > > ## name, \
> > > > + bs, options, flags, errp);
> > > >  \
> > > > +}  
> > > >  \
> > > > +   
> > > >  \
> > > > +static int block_crypto_create_ ## name(const char *filename,  
> > > >  \
> > > > +QemuOpts *opts,
> > > >  \
> > > > +Error **errp)  
> > > >  \
> > > > +{  
> > > >  \
> > > > +return block_crypto_create_generic(format, 
> > > >  \
> > > > +   filename, opts, errp);  
> > > >  \
> > > > +}  
> > > >  \
> > > > +   
> > > >  \
> > > > +BlockDriver bdrv_crypto_ ## name = {   
> > > >  \
> > > > +.format_name= #name,   
> > > >  \
> > > > +.instance_size  = sizeof(BlockCrypto), 
> > > >  \
> > > > +.bdrv_probe = block_crypto_probe_ ## name, 
> > > >  \
> > > > +.bdrv_open  = block_crypto_open_ ## name,  
> > > >  \
> > > > +.bdrv_close = block_crypto_close,  
> > > >  \
> > > > +.bdrv_create= block_crypto_create_ ## name,
> > > >  \
> > > > +.create_opts= _crypto_create_opts_ ## name,  
> > > >  \
> > > > +   
> > > >  \
> > > > +.bdrv_co_readv  = block_crypto_co_readv,   
> > > >  \
> > > > +.bdrv_co_writev = block_crypto_co_writev,  
> > > >  \
> > > > +.bdrv_getlength = block_crypto_getlength,  
> > > >  \
> > > > +}
> > > > +
> > > > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS);
> > > 
> > > Personally I really prefer a preprocessed version, for the ease of grep.
> > 
> > I'm not sure I understand what you mean by a preprocessed version - could
> > you expand on that.
> 
> I mean don't use macro concatenation and use plain symbols like in other block
> drivers.
> 
> BlockDriver bdrv_crypto_luks = {
> .format_name= "luks",
> .instance_size  = sizeof(BlockCrypto),
> .bdrv_probe = block_crypto_probe_luks,
> .bdrv_open  = block_crypto_open_luks,
> ...
> 
> mostly because it's easier to grep (or for refactoring with tools).
> 
> But I can't how repeatitive this would be (I can see the "don't repeat
> yourself" with your approach).  There is only one BLOCK_CRYPTO_DRIVER 

Re: [Qemu-devel] [PULL 00/17] Block patches

[Peter Maydell]

On 20 January 2016 at 16:24, Kevin Wolf  wrote:
> The following changes since commit 3db34bf64ab4f8797565dd8750003156c32b301d:
>
>   Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-peter' 
> into staging (2016-01-18 17:40:50 +)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to e9b155501da2638e4e319af9960d35da1bc8662b:
>
>   iotests: Test that throttle values ranges (2016-01-20 13:37:57 +0100)
>
> 
> Block layer patches
>
> 

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH 1/2] hmp: add hmp command for incremental backup

[Rudy Zhang]

Add hmp command for incremental backup in drive-backup.
It need a bitmap to backup data from drive-image to incremental image,
so before it need add bitmap for this device to track io.
Usage:
drive_backup [-n] [-f] device target [bitmap] [format]

Signed-off-by: Rudy Zhang 
---
 hmp-commands.hx |  5 +++--
 hmp.c   | 16 ++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..7378aaa 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1180,12 +1180,13 @@ ETEXI
 
 {
 .name   = "drive_backup",
-.args_type  = "reuse:-n,full:-f,device:B,target:s,format:s?",
-.params = "[-n] [-f] device target [format]",
+.args_type  = "reuse:-n,full:-f,device:B,target:s,bitmap:s?,format:s?",
+.params = "[-n] [-f] device target [bitmap] [format]",
 .help   = "initiates a point-in-time\n\t\t\t"
   "copy for a device. The device's contents are\n\t\t\t"
   "copied to the new image file, excluding data 
that\n\t\t\t"
   "is written after the command is started.\n\t\t\t"
+  "With bitmap will start incremental backup.\n\t\t\t"
   "The -n flag requests QEMU to reuse the image 
found\n\t\t\t"
   "in new-image-file, instead of recreating it from 
scratch.\n\t\t\t"
   "The -f flag requests QEMU to copy the whole 
disk,\n\t\t\t"
diff --git a/hmp.c b/hmp.c
index 54f2620..f8c33cd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1086,11 +1086,13 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
 {
 const char *device = qdict_get_str(qdict, "device");
 const char *filename = qdict_get_str(qdict, "target");
+const char *bitmap = qdict_get_try_str(qdict, "bitmap");
 const char *format = qdict_get_try_str(qdict, "format");
 bool reuse = qdict_get_try_bool(qdict, "reuse", false);
 bool full = qdict_get_try_bool(qdict, "full", false);
 enum NewImageMode mode;
 Error *err = NULL;
+enum MirrorSyncMode sync;
 
 if (!filename) {
 error_setg(, QERR_MISSING_PARAMETER, "target");
@@ -1098,6 +1100,17 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
 return;
 }
 
+if (full && bitmap) {
+error_setg(, "Parameter 'bitmap' if conflict with '-f'");
+hmp_handle_error(mon, );
+return;
+} else if (full)
+sync = MIRROR_SYNC_MODE_FULL;
+else if (bitmap)
+sync = MIRROR_SYNC_MODE_INCREMENTAL;
+else
+sync = MIRROR_SYNC_MODE_TOP;
+
 if (reuse) {
 mode = NEW_IMAGE_MODE_EXISTING;
 } else {
@@ -1105,8 +1118,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
 }
 
 qmp_drive_backup(device, filename, !!format, format,
- full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
- true, mode, false, 0, false, NULL,
+ sync, true, mode, false, 0, !!bitmap, bitmap,
  false, 0, false, 0, );
 hmp_handle_error(mon, );
 }
-- 
2.6.4

[Qemu-devel] [PATCH 0/2] block/hmp: add sereval hmp commands for incremental backup

[Rudy Zhang]

Add sereval hmp commands for incremental backup.
It need a bitmap to backup data from drive-image to incremental image,
so before it need add bitmap for this device to track io.

Usage:
   drive_backup [-n] [-f] device target [bitmap] [format]

Example:
   # qemu-img create -f qcow2 /path/incremental-1.qcow2 -b /path/full.qcow2
   (qemu) block_dirty_bitmap_add drive-virtio-disk0 bitmap0
   (qemu) drive-backup -n drive-virtio-disk0 /path/incremental-1.qcow2 bitmap0

Rudy Zhang (2):
  hmp: add hmp command for incremental backup
  hmp: add hmp commands dirty bitmap add/clear/remove'

 hmp-commands.hx | 47 ++--
 hmp.c   | 83 +++--
 hmp.h   |  3 +++
 3 files changed, 129 insertions(+), 4 deletions(-)

-- 
2.6.4

[Qemu-devel] [PATCH v2] target-i386/kvm: Hyper-V VMBus hypercalls blank handlers

[Andrey Smetanin]

Add Hyper-V VMBus hypercalls blank handlers which
just returns error code - HV_STATUS_INVALID_HYPERCALL_CODE.

Changes v2:
* use KVM_EXIT_HYPERV exit type

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Paolo Bonzini 
CC: Richard Henderson 
CC: Eduardo Habkost 
CC: "Andreas Färber" 
CC: Marcelo Tosatti 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: k...@vger.kernel.org

---
 target-i386/hyperv.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/target-i386/hyperv.c b/target-i386/hyperv.c
index e79b173..d3f3059 100644
--- a/target-i386/hyperv.c
+++ b/target-i386/hyperv.c
@@ -43,6 +43,18 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit 
*exit)
 return -1;
 }
 return 0;
+case KVM_EXIT_HYPERV_HCALL: {
+uint16_t code;
+
+code  = exit->u.hcall.input & 0x;
+switch (code) {
+case HV_X64_HCALL_POST_MESSAGE:
+case HV_X64_HCALL_SIGNAL_EVENT:
+default:
+exit->u.hcall.result = HV_STATUS_INVALID_HYPERCALL_CODE;
+return 0;
+}
+}
 default:
 return -1;
 }
-- 
2.4.3

[Qemu-devel] [PATCH v2 5/5] kvm/x86: Hyper-V VMBus hypercall userspace exit

[Andrey Smetanin]

The patch implements KVM_EXIT_HYPERV userspace exit
functionality for Hyper-V VMBus hypercalls:
HV_X64_HCALL_POST_MESSAGE, HV_X64_HCALL_SIGNAL_EVENT.

Changes v2:
* use KVM_EXIT_HYPERV for hypercalls

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 Documentation/virtual/kvm/api.txt |  6 ++
 arch/x86/kvm/hyperv.c | 29 ++---
 arch/x86/kvm/hyperv.h |  1 +
 arch/x86/kvm/x86.c|  5 +
 include/uapi/linux/kvm.h  |  6 ++
 5 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 053f613..1bf1a07 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3339,6 +3339,7 @@ EOI was received.
 
struct kvm_hyperv_exit {
 #define KVM_EXIT_HYPERV_SYNIC  1
+#define KVM_EXIT_HYPERV_HCALL  2
__u32 type;
union {
struct {
@@ -3347,6 +3348,11 @@ EOI was received.
__u64 evt_page;
__u64 msg_page;
} synic;
+   struct {
+   __u64 input;
+   __u64 result;
+   __u64 params[2];
+   } hcall;
} u;
};
/* KVM_EXIT_HYPERV */
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index e1daa8b..26ae973 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1093,6 +1093,14 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
case HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT:
kvm_vcpu_on_spin(vcpu);
break;
+   case HV_X64_HCALL_POST_MESSAGE:
+   case HV_X64_HCALL_SIGNAL_EVENT:
+   vcpu->run->exit_reason = KVM_EXIT_HYPERV;
+   vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
+   vcpu->run->hyperv.u.hcall.input = param;
+   vcpu->run->hyperv.u.hcall.params[0] = ingpa;
+   vcpu->run->hyperv.u.hcall.params[1] = outgpa;
+   return 0;
default:
res = HV_STATUS_INVALID_HYPERCALL_CODE;
break;
@@ -1100,12 +1108,19 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 
 set_result:
ret = res | (((u64)rep_done & 0xfff) << 32);
-   if (longmode) {
-   kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
-   } else {
-   kvm_register_write(vcpu, VCPU_REGS_RDX, ret >> 32);
-   kvm_register_write(vcpu, VCPU_REGS_RAX, ret & 0x);
-   }
-
+   kvm_hv_hypercall_set_result(vcpu, ret);
return 1;
 }
+
+void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
+{
+   bool longmode;
+
+   longmode = is_64_bit_mode(vcpu);
+   if (longmode)
+   kvm_register_write(vcpu, VCPU_REGS_RAX, result);
+   else {
+   kvm_register_write(vcpu, VCPU_REGS_RDX, result >> 32);
+   kvm_register_write(vcpu, VCPU_REGS_RAX, result & 0x);
+   }
+}
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 60eccd4..64a4a3b 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -52,6 +52,7 @@ int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 
*pdata);
 
 bool kvm_hv_hypercall_enabled(struct kvm *kvm);
 int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
+void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result);
 
 void kvm_hv_irq_routing_update(struct kvm *kvm);
 int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f53f5b1..e5c842b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6891,6 +6891,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
struct kvm_run *kvm_run)
} else
WARN_ON(vcpu->arch.pio.count || vcpu->mmio_needed);
 
+   if (unlikely(kvm_run->exit_reason == KVM_EXIT_HYPERV) &&
+   kvm_run->hyperv.type == KVM_EXIT_HYPERV_HCALL)
+   kvm_hv_hypercall_set_result(vcpu,
+   kvm_run->hyperv.u.hcall.result);
+
r = vcpu_run(vcpu);
 
 out:
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 9da9051..c5519a9 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -157,6 +157,7 @@ struct kvm_s390_skeys {
 
 struct kvm_hyperv_exit {
 #define KVM_EXIT_HYPERV_SYNIC

Re: [Qemu-devel] [PATCH v2 12/17] qcow2: convert QCow2 to use QCryptoBlock for encryption

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 05:54:23PM +0800, Fam Zheng wrote:
> On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > This converts the qcow2 driver to make use of the QCryptoBlock
> > APIs for encrypting image content. As well as continued support
> > for the legacy QCow2 encryption format, the appealing benefit
> > is that it enables support for the LUKS format inside qcow2.
> 
> FWIW, with today's QEMU, it's possible to stack format drivers on top of each
> other.  In other words, even without this patch, we can make LUKS driver
> encrypt/decrypt the qcow2 payload, while keeping them completely orthogonal.

Yep, that is certainly possible, and it is what is intended for using
LUKS with RBD, iSCSI, & other network drivers.

I think there is value in having LUKS integrated directly into qcow2
though. It means that given a qcow2 file you can 100% reliably
distinguish between a file created with the intention of QEMU managing
the LUKS encryption, from a file where the guest OS happens to have
set up LUKS encryption in its virtual disk. If you don't have this,
then given a random qcow2 file, you have to probe to see if LUKS is
present or not. Given the security issues we've had in the past with
raw images being turned into qcow2 images by a malicious guest writing
a qcow2 header, I feel that having explicitly integration LUKS support
in QCow is worthwhile as a concept.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 03/17] crypto: add support for PBKDF2 algorithm

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 02:59:24PM +0800, Fam Zheng wrote:
> On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > The LUKS data format includes use of PBKDF2 (Password-Based
> > Key Derivation Function). The Nettle library can provide
> > an implementation of this, but we don't want code directly
> > depending on a specific crypto library backend. Introduce
> > a include/crypto/pbkdf.h header which defines a QEMU
> > API for invoking PBKDK2. The initial implementations are
> > backed by nettle & gcrypt, which are commonly available
> > with distros shipping GNUTLS.
> > 
> > The test suite data is taken from the cryptsetup codebase
> > under the LGPLv2.1+ license. This merely aims to verify
> > that whatever backend we provide for this function in QEMU
> > will comply with the spec.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  crypto/Makefile.objs  |   6 +-
> >  crypto/pbkdf-gcrypt.c |  65 
> >  crypto/pbkdf-nettle.c |  64 
> >  crypto/pbkdf-stub.c   |  41 +
> >  crypto/pbkdf.c|  68 +
> >  include/crypto/pbkdf.h| 152 +++
> >  tests/.gitignore  |   1 +
> >  tests/Makefile|   2 +
> >  tests/test-crypto-pbkdf.c | 378 
> > ++
> >  9 files changed, 776 insertions(+), 1 deletion(-)
> >  create mode 100644 crypto/pbkdf-gcrypt.c
> >  create mode 100644 crypto/pbkdf-nettle.c
> >  create mode 100644 crypto/pbkdf-stub.c
> >  create mode 100644 crypto/pbkdf.c
> >  create mode 100644 include/crypto/pbkdf.h
> >  create mode 100644 tests/test-crypto-pbkdf.c
> > 
> > +int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
> > +   const uint8_t *key, size_t nkey,
> > +   const uint8_t *salt, size_t nsalt,
> > +   unsigned int iterations,
> > +   uint8_t *out, size_t nout,
> > +   Error **errp)
> > +{
> > +static const int hash_map[QCRYPTO_HASH_ALG__MAX] = {
> > +[QCRYPTO_HASH_ALG_MD5] = GCRY_MD_MD5,
> > +[QCRYPTO_HASH_ALG_SHA1] = GCRY_MD_SHA1,
> > +[QCRYPTO_HASH_ALG_SHA256] = GCRY_MD_SHA256,
> > +};
> > +int ret;
> > +
> > +if (hash > G_N_ELEMENTS(hash_map)) {
> 
> Do you want ">="?

Yes.

> 
> > +error_setg(errp, "Unexpected hash algorithm %d", hash);
> > +return -1;
> > +}
> > +
> > +ret = gcry_kdf_derive(key, nkey, GCRY_KDF_PBKDF2,
> > +  hash_map[hash],
> > +  salt, nsalt, iterations,
> > +  nout, out);
> 
> We go ahead with QCRYPTO_HASH_ALG_MD5 here, but we didn't accept it in
> qcrypto_pbkdf2_supports, is that a mistake?

Yes, I should have reported MD5 in the earlier function, since
gcrypt allows that.

> > diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c
> > new file mode 100644
> > index 000..71f96cd
> > --- /dev/null
> > +++ b/crypto/pbkdf.c
> > @@ -0,0 +1,68 @@
> > +/*
> > + * QEMU Crypto PBKDF support (Password-Based Key Derivation Function)
> > + *
> > + * Copyright (c) 2015-2016 Red Hat, Inc.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see 
> > .
> > + *
> > + */
> > +
> > +#include "crypto/pbkdf.h"
> > +#include 
> > +
> > +
> > +int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
> > +   const uint8_t *key, size_t nkey,
> > +   const uint8_t *salt, size_t nsalt,
> > +   Error **errp)
> > +{
> > +uint8_t out[32];
> > +int iterations = (1 << 15);
> 
> To be safe from overflow, I'd use at least 64 bits to do the math, just in 
> case
> that some machine is too good at computing this stuff. :)

Hmm, probably need to change the return type to be 64 bit too then

> 
> > +struct rusage start, end;
> > +unsigned long long delta;
> > +
> > +while (1) {
> > +if (getrusage(RUSAGE_THREAD, ) < 0) {
> > +error_setg_errno(errp, errno, "Unable to get thread CPU 
> > usage");
> > +return -1;
> > +}
> > +if (qcrypto_pbkdf2(hash,
> > +   key, nkey,
> > +   salt, nsalt,
> > +   iterations,
> > +   out, sizeof(out),
> > + 

[Qemu-devel] [PATCH v2 1/3] target-arm: Apply S2 MMU startlevel table size check to AArch64

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

The S2 starting level table size check applies to both AArch32
and AArch64. Move it to common code.

Reviewed-by: Alex Bennée 
Signed-off-by: Edgar E. Iglesias 
---
 target-arm/helper.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index f956b67..8aedce9 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6581,11 +6581,19 @@ typedef enum {
 static bool check_s2_startlevel(ARMCPU *cpu, bool is_aa64, int level,
 int inputsize, int stride)
 {
+const int grainsize = stride + 3;
+int startsizecheck;
+
 /* Negative levels are never allowed.  */
 if (level < 0) {
 return false;
 }
 
+startsizecheck = inputsize - ((3 - level) * stride + grainsize);
+if (startsizecheck < 1 || startsizecheck > stride + 4) {
+return false;
+}
+
 if (is_aa64) {
 unsigned int pamax = arm_pamax(cpu);
 
@@ -6609,20 +6617,12 @@ static bool check_s2_startlevel(ARMCPU *cpu, bool 
is_aa64, int level,
 g_assert_not_reached();
 }
 } else {
-const int grainsize = stride + 3;
-int startsizecheck;
-
 /* AArch32 only supports 4KB pages. Assert on that.  */
 assert(stride == 9);
 
 if (level == 0) {
 return false;
 }
-
-startsizecheck = inputsize - ((3 - level) * stride + grainsize);
-if (startsizecheck < 1 || startsizecheck > stride + 4) {
-return false;
-}
 }
 return true;
 }
-- 
1.9.1

Re: [Qemu-devel] [PATCH v5 0/4] Extend TPM support with a QEMU-external TPM

[Dr. David Alan Gilbert]

* Stefan Berger (stef...@us.ibm.com) wrote:
> Stefan Berger/Watson/IBM wrote on 01/20/2016 02:51:58 PM:
> 
> > "Daniel P. Berrange"  wrote on 01/20/2016 10:42:09 
> AM:
> > 
> > > 
> > > On Wed, Jan 20, 2016 at 10:23:50AM -0500, Stefan Berger wrote:
> > > > "Daniel P. Berrange"  wrote on 01/20/2016 
> 09:58:39 
> > > > AM:
> > > > 
> > > > 
> > > > > Subject: Re: [Qemu-devel] [PATCH v5 0/4] Extend TPM support with a 
> 
> > > > > QEMU-external TPM
> > > > > 
> > > > > On Mon, Jan 04, 2016 at 10:23:18AM -0500, Stefan Berger wrote:
> > > > > > The following series of patches extends TPM support with an
> > > > > > external TPM that offers a Linux CUSE (character device in 
> userspace)
> > > > > > interface. This TPM lets each VM access its own private vTPM.
> > > > > 
> > > > > What is the backing store for this vTPM ? Are the vTPMs all
> > > > > multiplexed onto the host's physical TPM or is there something
> > > > > else going on ?
> > > > 
> > > > The vTPM writes its state into a plain file. In case the user 
> started the 
> > > > vTPM, the user gets to choose the directory. In case of libvirt, 
> libvirt 
> > > > sets up the directory and starts the vTPM with the directory as a 
> > > > parameter. The expectation for VMs (also containers) is that each VM 
> can 
> > > > use the full set of TPM commands with the vTPM and due to how the 
> TPM 
> > > > works, it cannot use the hardware TPM for that. SeaBIOS has 
> beenextended 
> > > > with TPM 1.2 support and initializes the vTPM in the same way it 
> would 
> > > > initialize a hardware TPM.
> > > 
> > > So if its using a plain file, then when snapshotting VMs we have to
> > > do full copies of the file and keep them all around in sync with
> > > the disk snapshots. By not having this functionality in QEMU we don't
> > > immediately have a way to use qcow2 for the vTPM file backing store
> > > to deal with snapshot management. The vTPM needs around snapshotting
> > > feel fairly similar to the NVRAM needs, so it would be desiralbe to
> > > have a ability to do a consistent thing for both.
> > 
> > The plain file serves as the current state of the TPM. In case of 
> > migration, suspend, snapshotting, the vTPM state blobs are retrieved
> > from the vTPM using ioctls and in case of a snapshot they are 
> > written into the QCoW2. Upon resume the state blobs are set in the 
> > vTPM. I is working as it is.
> 
> There is one issue in case of resume of a snapshot. If the permanent state 
> of the TPM is modified during snapshotting, like ownership is taken of the 
> TPM, the state, including the owner password, is written into the plain 
> file. Then the VM is shut down. Once it is restarted (not a resume from a 
> snapshot), the TPM's state will be relected by what was done during the 
> run of that snapshot. So this is likely undesirable. Now the only way 
> around this seems to be that one needs to know the reason for why the 
> state blobs were pushed into the vTPM. In case of a snapshot, the writing 
> of the permanent state into a file may need to be suppressed, while on a 
> VM resume and resume from migration operation it needs to be written into 
> the TPM's state file.

I don't understand that; are you saying that the ioctl's dont provide all
the information that's included in the state file?

Dave

> 
>Stefan
> 
> > 
> >Stefan
> > 
> > 
> > > 
> > > Regards,
> > > Daniel
> > > -- 
> > > |: http://berrange.com  -o-
> http://www.flickr.com/photos/dberrange/:|
> > > |: http://libvirt.org  -o- 
> http://virt-manager.org:|
> > > |: http://autobuild.org   -o- 
> http://search.cpan.org/~danberr/:|
> > > |: http://entangle-photo.org   -o-   
> http://live.gnome.org/gtk-vnc:|
> > > 
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Richard W.M. Jones]

On Thu, Jan 21, 2016 at 02:42:02PM +0300, Michael Tokarev wrote:
> 21.01.2016 14:37, Richard W.M. Jones wrote:
> 
> > I'm afraid I gave up on this -- did give it my best.  It turns out
> > that the machine that I thought supported UEFI boot does not.  I'll
> > keep an eye out for such a machine and test this in future.
> 
> BTW, why do you guys refer to UEFI boot all the time?  The whole thing
> is equally useful on traditional BIOS-booting machines too.  Windows7
> uses the same mechanism (looking at RSDT) for offline activation no
> matter if it is EFI system or not.
> 
> When I did first version of my patch (based on someone else's version,
> but details escapes my memory already), there was no UEFI support for
> qemu whatsoever, and it allowed me to use my OEM version of Windows7
> inside a qemu virtual machine without second activation.

Could really do with detailed steps to reproduce the problem.  We have
never observed it, and I know next to nothing about how "activation"
works.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH v1 2/3] target-arm: Make pamax an argument to check_s2_startlevel

[Alex Bennée]

Edgar E. Iglesias  writes:

> From: "Edgar E. Iglesias" 
>
> Make pamax an argument to check_s2_startlevel in preparation
> for future reuse.
>
> No functional change.
>
> Signed-off-by: Edgar E. Iglesias 
> ---
>  target-arm/helper.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 8aedce9..4abeb4d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6579,7 +6579,8 @@ typedef enum {
>   * Returns true if the suggested starting level is OK and false otherwise.
>   */
>  static bool check_s2_startlevel(ARMCPU *cpu, bool is_aa64, int level,
> -int inputsize, int stride)
> +int inputsize, int stride,
> +unsigned int pamax)

I'm not convinced about this as we only use pamax in one leg and we can
get it via *cpu when needed. If it will eventually be used you would
also have to update the docstring above.

>  {
>  const int grainsize = stride + 3;
>  int startsizecheck;
> @@ -6595,8 +6596,6 @@ static bool check_s2_startlevel(ARMCPU *cpu, bool 
> is_aa64, int level,
>  }
>
>  if (is_aa64) {
> -unsigned int pamax = arm_pamax(cpu);
> -
>  switch (stride) {
>  case 13: /* 64KB Pages.  */
>  if (level == 0 || (level == 1 && pamax <= 42)) {
> @@ -6808,6 +6807,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
> target_ulong address,
>   * VTCR_EL2.SL0 field (whose interpretation depends on the page size)
>   */
>  int startlevel = extract32(tcr->raw_tcr, 6, 2);
> +unsigned int pamax = arm_pamax(cpu);
>  bool ok;
>
>  if (va_size == 32 || stride == 9) {
> @@ -6820,7 +6820,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
> target_ulong address,
>
>  /* Check that the starting level is valid. */
>  ok = check_s2_startlevel(cpu, va_size == 64, level,
> - inputsize, stride);
> + inputsize, stride, pamax);
>  if (!ok) {
>  /* AArch64 reports these as level 0 faults.
>   * AArch32 reports these as level 1 faults.


--
Alex Bennée

Re: [Qemu-devel] [PATCH v17 6/9] qmp/hmp: add set-vm-generation-id commands

[Igor Mammedov]

On Wed, 20 Jan 2016 09:48:41 -0700
Eric Blake  wrote:

> On 01/19/2016 06:06 AM, Igor Mammedov wrote:
> > Add set-vm-generation-id command to set Virtual Machine
> > Generation ID counter.
> > 
> > QMP command example:
> > { "execute": "set-vm-generation-id",
> >   "arguments": {
> >   "uuid": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
> >   }
> > }
> > 
> > HMP command example:
> > set-vm-generation-id 324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87
> > 
> > Signed-off-by: Igor Mammedov 
> > ---
> 
> > +++ b/hmp.c
> > @@ -2384,3 +2384,15 @@ void hmp_info_vm_generation_id(Monitor *mon,
> > const QDict *qdict) }
> >  qapi_free_UuidInfo(info);
> >  }
> > +
> > +void hmp_set_vm_generation_id(Monitor *mon, const QDict *qdict)
> > +{
> > +Error *errp = NULL;
> > +const char *uuid = qdict_get_str(qdict, "uuid");
> > +
> > +qmp_set_vm_generation_id(uuid, );
> > +if (errp != NULL) {
> 
> I might have written 'if (errp)', but that's cosmetic style.
> 
> > +++ b/qapi-schema.json
> > @@ -4090,3 +4090,12 @@
> >  # Since 2.6
> >  ##
> >  { 'command': 'query-vm-generation-id', 'returns': 'UuidInfo' }
> > +
> > +##
> > +# @set-vm-generation-id
> > +#
> > +# Set Virtual Machine Generation ID
> > +#
> 
> Missing documentation of the @uuid argument.
> 
> > +# Since 2.6
> > +##
> > +{ 'command': 'set-vm-generation-id', 'data': { 'uuid': 'str' } }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 9408a3d..306082f 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -4814,3 +4814,25 @@ Example:
> >  
> >  -> { "execute": "query-vm-generation-id" }
> >  <- {"return": {"UUID": "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"}}
> 
> Annoying that the set uses 'uuid' and the query responds with 'UUID';
> I prefer the lowercase version in new API, so maybe patch 5/9 should
> create a new return type with desirable spelling rather than reusing
> the older type.
ok, I'll add a new type.

> 
> 
> > +SQMP
> > +Set Virtual Machine Generation ID counter
> > +-
> > +
> > +Arguments:
> > +
> > +- "UUID": counter ID in UUID string representation (json-string)"
> 
> wrong case, per the example.
> 
> > +
> > +Example:
> > +
> > +-> { "execute": "set-vm-generation-id" ,
> > + "arguments": { "uuid":
> > "324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87" } } +<- {"return": {}}
> 

Re: [Qemu-devel] [PULL v1 0/5] I/O channels fixes

[Peter Maydell]

On 20 January 2016 at 12:01, Daniel P. Berrange  wrote:
> The following changes since commit 3db34bf64ab4f8797565dd8750003156c32b301d:
>
>   Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-peter' 
> into staging (2016-01-18 17:40:50 +)
>
> are available in the git repository at:
>
>   git://github.com/berrange/qemu tags/pull-io-next-2016-01-20-1
>
> for you to fetch changes up to ccf1e2dcd6091eea1fc2341c63201aa1a6094978:
>
>   io: use memset instead of { 0 } for initializing array (2016-01-20 11:31:01 
> +)
>
> 
> I/O channels fixes 2016/01/20 v1
>
> 
> Daniel P. Berrange (5):
>   io: fix sign of errno value passed to error report
>   io: increment counter when killing off subcommand
>   io: some fixes to handling of /dev/null when running commands
>   io: fix description of @errp parameter initialization
>   io: use memset instead of { 0 } for initializing array

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v1 3/3] target-arm: Implement the S2 MMU inputsize < pamax check

[Alex Bennée]

Edgar E. Iglesias  writes:

> On Wed, Jan 20, 2016 at 02:49:40PM +0100, Edgar E. Iglesias wrote:
>> From: "Edgar E. Iglesias" 
>>
>> Implement the inputsize < pamax check for Stage 2 translations.
>> We have multiple choices for how to respond to errors and
>> choose to fault.
>>
>> Signed-off-by: Edgar E. Iglesias 
>> ---
>>  target-arm/helper.c | 15 +++
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 4abeb4d..e1fa209 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -6808,7 +6808,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
>> target_ulong address,
>>   */
>>  int startlevel = extract32(tcr->raw_tcr, 6, 2);
>>  unsigned int pamax = arm_pamax(cpu);
>> -bool ok;
>> +bool ok = true;
>>
>>  if (va_size == 32 || stride == 9) {
>>  /* AArch32 or 4KB pages */
>> @@ -6818,9 +6818,16 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
>> target_ulong address,
>>  level = 3 - startlevel;
>>  }
>>
>> -/* Check that the starting level is valid. */
>> -ok = check_s2_startlevel(cpu, va_size == 64, level,
>> - inputsize, stride, pamax);
>> +if (inputsize > pamax &&
>> +(arm_el_is_aa64(env, 1) || inputsize > 40)) {
>
> I realized that this check should only be done for AArch64...
> Will fix that for v2.
>
> Something like the following:
>
> if (arm_el_is_aa64(env, el) &&
> inputsize > pamax &&
> (arm_el_is_aa64(env, 1) || inputsize > 40)) {
> /* We have multiple choices but choose to fault.  */
> ok = false;
> }
>

OK, I'll await the next revision.

>
> Cheers,
> Edgar
>
>
>> +/* We have multiple choices but choose to fault.  */
>> +ok = false;
>> +}
>> +if (ok) {
>> +/* Check that the starting level is valid. */
>> +ok = check_s2_startlevel(cpu, va_size == 64, level,
>> + inputsize, stride, pamax);
>> +}
>>  if (!ok) {
>>  /* AArch64 reports these as level 0 faults.
>>   * AArch32 reports these as level 1 faults.
>> --
>> 1.9.1
>>


--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 1/1] arm_gic: Update ID registers based on revision

[Peter Maydell]

On 21 January 2016 at 00:18, Alistair Francis
 wrote:
> Update the GIC ID registers (registers above 0xfe0) based on the GIC
> revision instead of using the sames values for all GIC implementations.
>
> Signed-off-by: Alistair Francis 
> Tested-by: Sören Brinkmann 
> ---
> V3:
>  - Add an assert()
> V2:
>  - Update array indexing to match new values
>  - Check the maximum value of offset as well
>
>  hw/intc/arm_gic.c | 35 ++-
>  1 file changed, 30 insertions(+), 5 deletions(-)



Applied to target-arm.next, thanks.

-- PMM

[Qemu-devel] virtio ring layout changes for optimal single-stream performance

[Michael S. Tsirkin]

Hi all!
I have been experimenting with alternative virtio ring layouts,
in order to speed up single stream performance.

I have just posted a benchmark I wrote for the purpose, and a (partial)
alternative layout implementation.  This achieves 20-40% reduction in
virtio overhead in the (default) polling mode.

http://article.gmane.org/gmane.linux.kernel.virtualization/26889

The layout is trying to be as simple as possible, to reduce
the number of cache lines bouncing between CPUs.

For benchmarking, the idea is to emulate virtio in user-space,
artificially adding overhead for e.g. signalling to match what happens
in case of a VM.

I'd be very curious to get feedback on this, in particular, some people
discussed using vectored operations to format virtio ring - would it
conflict with this work?

You are all welcome to post enhancements or more layout alternatives as
patches.

TODO:
- documentation+discussion of interaction with CPU caching
- thorough benchmarking of different configurations/hosts
- experiment with event index replacements
- better emulate vmexit/vmentry cost overhead
- virtio spec proposal

Thanks!
-- 
MST

Re: [Qemu-devel] [PATCH v2 12/17] qcow2: convert QCow2 to use QCryptoBlock for encryption

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 09:56:11PM +0800, Fam Zheng wrote:
> On Thu, 01/21 10:50, Daniel P. Berrange wrote:
> > On Thu, Jan 21, 2016 at 05:54:23PM +0800, Fam Zheng wrote:
> > > On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > > > This converts the qcow2 driver to make use of the QCryptoBlock
> > > > APIs for encrypting image content. As well as continued support
> > > > for the legacy QCow2 encryption format, the appealing benefit
> > > > is that it enables support for the LUKS format inside qcow2.
> > > 
> > > FWIW, with today's QEMU, it's possible to stack format drivers on top of 
> > > each
> > > other.  In other words, even without this patch, we can make LUKS driver
> > > encrypt/decrypt the qcow2 payload, while keeping them completely 
> > > orthogonal.
> > 
> > Yep, that is certainly possible, and it is what is intended for using
> > LUKS with RBD, iSCSI, & other network drivers.
> > 
> > I think there is value in having LUKS integrated directly into qcow2
> > though. It means that given a qcow2 file you can 100% reliably
> > distinguish between a file created with the intention of QEMU managing
> > the LUKS encryption, from a file where the guest OS happens to have
> > set up LUKS encryption in its virtual disk. If you don't have this,
> > then given a random qcow2 file, you have to probe to see if LUKS is
> > present or not. Given the security issues we've had in the past with
> > raw images being turned into qcow2 images by a malicious guest writing
> > a qcow2 header, I feel that having explicitly integration LUKS support
> > in QCow is worthwhile as a concept.
> 
> Yes, I'm not objecting to explicit (read: mandatory) encryption in qcow2 in 
> any
> way, and extending the format spec for LUKS is a good thing.
> 
> I mentioned stacked drivers because I wonder how good we can do in reusing
> block/crypto.c code to achieve this (to save 500+ new code in qcow2). For
> example I have a rough idea along this:
> 
> * In qcow2 spec, define type "2" of crypt_method for LUKS encryption, and 
> state
>   that if this method is used, the payload must be LUKS format as defined in
>   cryptsetup's docs/on-disk-format.pdf, and driver will take care of
>   encrypting/decrypting data in a way that is transparent to guest.
> 
> * In qcow2_open, if header's crypt_method is LUKS, set a special flag in the
>   BlockDriverState to indicate that block layer should create an overlay 
> crypto
>   driver (which will be luks in this case), on top of this BDS. To do this we
>   need some modification to bdrv_open.
> 
> * When qcow2_open is done, block layer will then instantiate a LUKS driver,
>   which backed by the qcow2 BDS (as the BDS.file). This LUKS BDS will then be
>   attached to guest device.
> 
> * From the guest PoV, R/W reqs are served as if it's an ordinary qcow2; from
>   LUKS driver's PoV, it works properly formatted luks data, which is backed by
>   whatever it doesn't care, be it an iscsi target, rbd block, local file, or
>   a qcow2 image that has metadata; from the qcow2 driver's PoV, it only made
>   sure that a LUKS driver covers itself, at instantiation time.  Everything
>   else operates the same way as a non-encrypted qcow2.
> 
> * Of course, qcow2_create would need some similar changes to the block layer 
> as
>   to bdrv_open, but that shouldn't be too hard.
> 
> Makes sense?

Hmm, interesting idea. So essentially we'd be having a format driver
be able to inform the generic block code to insert an encryption
layer above it.  The downside is that it leaks details about use
of encryption out into the generic block code. The upside is that
it reduces code in the qcow2 impl.  I do wonder if that would lead
to more or less code overall, or whether the generic block layer
additions would cancel out the removals from qcow2.

BTW, we can probably reduce the amount of code in qcow2 regardless,
as about 1/2 of the addition is related to handling of the QemuOpts
<-> QCryptoBlockOpenOptions conversion, and I could share that
with the block/crypto.c driver to a greater extent than I do now.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

[Qemu-devel] [PATCH v2 2/5] drivers/hv: Move VMBus hypercall codes into Hyper-V UAPI header

[Andrey Smetanin]

VMBus hypercall codes inside Hyper-V UAPI header will
be used by QEMU to implement VMBus host devices support.

Signed-off-by: Andrey Smetanin 
Acked-by: K. Y. Srinivasan 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 2 ++
 drivers/hv/hv.c| 4 ++--
 drivers/hv/hyperv_vmbus.h  | 6 --
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 0c50fab..bc1c8a9 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -227,6 +227,8 @@
 
 /* Declare the various hypercall operations. */
 #define HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT 0x0008
+#define HV_X64_HCALL_POST_MESSAGE  0x005c
+#define HV_X64_HCALL_SIGNAL_EVENT  0x005d
 
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ENABLE 0x0001
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT  12
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 11bca51..5ce2dec 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -331,7 +331,7 @@ int hv_post_message(union hv_connection_id connection_id,
aligned_msg->payload_size = payload_size;
memcpy((void *)aligned_msg->payload, payload, payload_size);
 
-   status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL);
+   status = hv_do_hypercall(HV_X64_HCALL_POST_MESSAGE, aligned_msg, NULL);
 
put_cpu();
return status & 0x;
@@ -348,7 +348,7 @@ int hv_signal_event(void *con_id)
 {
u64 status;
 
-   status = hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL);
+   status = hv_do_hypercall(HV_X64_HCALL_SIGNAL_EVENT, con_id, NULL);
 
return status & 0x;
 }
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 4ebc796..2f8c0f4 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -256,12 +256,6 @@ struct hv_monitor_page {
u8 rsvdz4[1984];
 };
 
-/* Declare the various hypercall operations. */
-enum hv_call_code {
-   HVCALL_POST_MESSAGE = 0x005c,
-   HVCALL_SIGNAL_EVENT = 0x005d,
-};
-
 /* Definition of the hv_post_message hypercall input structure. */
 struct hv_input_post_message {
union hv_connection_id connectionid;
-- 
2.4.3

[Qemu-devel] [PATCH v2 2/3] target-arm: Make pamax an argument to check_s2_startlevel

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Make pamax an argument to check_s2_startlevel in preparation
for future reuse.

No functional change.

Signed-off-by: Edgar E. Iglesias 
---
 target-arm/helper.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8aedce9..4abeb4d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6579,7 +6579,8 @@ typedef enum {
  * Returns true if the suggested starting level is OK and false otherwise.
  */
 static bool check_s2_startlevel(ARMCPU *cpu, bool is_aa64, int level,
-int inputsize, int stride)
+int inputsize, int stride,
+unsigned int pamax)
 {
 const int grainsize = stride + 3;
 int startsizecheck;
@@ -6595,8 +6596,6 @@ static bool check_s2_startlevel(ARMCPU *cpu, bool 
is_aa64, int level,
 }
 
 if (is_aa64) {
-unsigned int pamax = arm_pamax(cpu);
-
 switch (stride) {
 case 13: /* 64KB Pages.  */
 if (level == 0 || (level == 1 && pamax <= 42)) {
@@ -6808,6 +6807,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
  * VTCR_EL2.SL0 field (whose interpretation depends on the page size)
  */
 int startlevel = extract32(tcr->raw_tcr, 6, 2);
+unsigned int pamax = arm_pamax(cpu);
 bool ok;
 
 if (va_size == 32 || stride == 9) {
@@ -6820,7 +6820,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, 
target_ulong address,
 
 /* Check that the starting level is valid. */
 ok = check_s2_startlevel(cpu, va_size == 64, level,
- inputsize, stride);
+ inputsize, stride, pamax);
 if (!ok) {
 /* AArch64 reports these as level 0 faults.
  * AArch32 reports these as level 1 faults.
-- 
1.9.1

Re: [Qemu-devel] [PATCH 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Michael Tokarev]

21.01.2016 14:37, Richard W.M. Jones wrote:

> I'm afraid I gave up on this -- did give it my best.  It turns out
> that the machine that I thought supported UEFI boot does not.  I'll
> keep an eye out for such a machine and test this in future.

BTW, why do you guys refer to UEFI boot all the time?  The whole thing
is equally useful on traditional BIOS-booting machines too.  Windows7
uses the same mechanism (looking at RSDT) for offline activation no
matter if it is EFI system or not.

When I did first version of my patch (based on someone else's version,
but details escapes my memory already), there was no UEFI support for
qemu whatsoever, and it allowed me to use my OEM version of Windows7
inside a qemu virtual machine without second activation.

Thanks,

/mjt

Re: [Qemu-devel] [PULL 0/6] Convert qemu-socket to use QAPI exclusively, update MAINTAINERS

[Peter Maydell]

On 20 January 2016 at 06:53, Gerd Hoffmann  wrote:
>   Hi,
>
> Update for the qemu socket code, switching it over to QAPI structs.
> Also adds MAINTAINERS entry for qemu socket code (Dan, Paolo, /me).
>
> please pull,
>   Gerd
>
> The following changes since commit 4aaddc2976bff1918edcd53900b647dde473dd4d:
>
>   Merge remote-tracking branch 'remotes/mcayland/tags/qemu-sparc-signed' into 
> staging (2016-01-18 09:33:36 +)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-socket-20160120-1
>
> for you to fetch changes up to 3f0230e926b45905efb597824f59fdf5a4f3da08:
>
>   vnc: distiguish between ipv4/ipv6 omitted vs set to off (2016-01-19 
> 15:41:01 +0100)
>
> 
> Convert qemu-socket to use QAPI exclusively, update MAINTAINERS.
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v17 0/9] Virtual Machine Generation ID

[Igor Mammedov]

On Wed, 20 Jan 2016 15:20:12 +0100
Laszlo Ersek  wrote:

> On 01/20/16 10:18, Igor Mammedov wrote:
> > On Tue, 19 Jan 2016 17:49:30 +0100
> > Laszlo Ersek  wrote:
> > 
> >> On 01/19/16 15:48, Igor Mammedov wrote:
> >>> Here is SSDT ASL diff with vmgenid device present that
> >>> Laszlo's asked for:
> >>>
> >>> @@ -135,6 +135,11 @@ DefinitionBlock
> >>> ("tests/acpi-test-data/pc/SSDT.aml", "SSDT", 1, "BOCHS ",
> >>> "BXPCS }) }
> >>>  
> >>> +Method (\_GPE._E00, 0, NotSerialized)  // _Exx:
> >>> Edge-Triggered GPE
> >>> +{
> >>> +Notify (\_SB.PCI0.VGEN, 0x80) // Status Change
> >>> +}
> >>> +  
> >>
> >> Thanks a lot! I have one comment for this: I think _E00 cannot be
> >> used. Please see the argument in my earlier (now obsolete) patch:
> >>
> >>   [PATCH FYI 02/13] hw/acpi: add i386 callbacks for injecting GPE
> >> 04 when the VMGENID changes
> >>
> >>   http://thread.gmane.org/gmane.comp.emulators.qemu/357940/focus=361705
> >>
> >> Thanks!
> >> Laszlo
> > It should be ok to use _E00 per spec since it's GPE event.
> > 
> > Here is quote from spec to what you were referring in your patch:
> > 
> > '(A query response of 0 from the embedded controller is
> > reserved for “no outstanding events.”)'
> > 
> > That limits 0 bit handler limitation only to 'embedded controller'
> > which is handled by _QXX methods.
> 
> Okay.
> 
> My other remark / question is then, do you think that having *both*
> _L00 and _E00 is valid?
> 
> The section under
> 
> 5.6.4.1.1 Queuing the Matching Control Method for Execution
> 
> seems to suggest that it's either-or; you can have at most one kind of
> handler (either _Exx for edge triggered, or _Lxx for level triggered).
> 
> _L00 is already present in the DSDT; see build_dsdt():
> 
> aml_append(scope, aml_method("_L00", 0, AML_NOTSERIALIZED));
> 
> and this series adds _E00.
> 
> ... Hm, wait, it's okay -- your "[PATCH v17 3/9] pc: add a Virtual
> Machine Generation ID device" removes the _L00 method. Cool.
> 
> (I just wonder why the ASL diff you posted didn't show this... I guess
> because that diff was for the SSDT only, and the _L00 method was
> removed from the DSDT.)
yep, and I'm working on merging both tables which should allow to
simplify/reduce generated ASL code and it's maintenance in future.

>  
> Thanks!
> Laszlo
> 
> > 
> >>
> >>
> >>>  Scope (\_SB)
> >>>  {
> >>>  Device (PCI0.PRES)
> >>> @@ -703,6 +708,28 @@ DefinitionBlock
> >>> ("tests/acpi-test-data/pc/SSDT.aml", "SSDT", 1, "BOCHS ", "BXPCS
> >>> DVNT (PCIU, One) DVNT (PCID, 0x03)
> >>>  }
> >>> +
> >>> +Device (VGEN)
> >>> +{
> >>> +Name (_HID, "QEMU0003")  // _HID: Hardware ID
> >>> +Name (_CID, "VM_Gen_Counter")  // _CID:
> >>> Compatible ID
> >>> +Name (_DDN, "VM_Gen_Counter")  // _DDN: DOS
> >>> Device Name
> >>> +Name (ADDR, Package (0x02)
> >>> +{
> >>> +0xFEBF, 
> >>> +Zero
> >>> +})
> >>> +Name (_CRS, ResourceTemplate ()  // _CRS:
> >>> Current Resource Settings
> >>> +{
> >>> +QWordMemory (ResourceProducer, PosDecode,
> >>> MinFixed, MaxFixed, Cacheable, ReadWrite,
> >>> +0x, // Granularity
> >>> +0xFEBF, // Range Minimum
> >>> +0xFEBF0FFF, // Range Maximum
> >>> +0x, // Translation Offset
> >>> +0x1000, // Length
> >>> +,, , AddressRangeMemory, TypeStatic)
> >>> +})
> >>> +}
> >>>  }
> >>>  }
> >>>  }
> >>>
> >>>
> >>> 'make V=1 check' doesn't show it since I've forgot to extend
> >>> bios-tables-test with vmgenid variant, but it should be
> >>> a separate patch anyway and I'd prefer to do it after
> >>> I merge DSDT with SSDT, which will reduce number of
> >>> test blobs we keep in tree along with other benefits.
> >>>
> >>> On Tue, 19 Jan 2016 14:06:20 +0100
> >>> Igor Mammedov  wrote:
> >>>   
>  It's respin of v14* series which uses a PCI BAR to map
>  VGID page in guest AS.
> 
>  Changes since v14:
>    - statically reserve used BAR resources in SSDT, so
>  that Windows won't claim them during PCI rebalancing
>    - support VGID page in high mem in addition to low mem
>    - add QMP/HMP interfaces to get/set VM Generation ID
>    - do not consume a PCI slot by default and attach
>  vmgenid device as a function of multifuction
>  ISA bridge.
>    - allow only one vmgenid device instance
> 
> 
>  Tested with WS2012R2x64.
>  Git tree for testing:
>  https://github.com/imammedo/qemu.git vmgenid_v17
> 
>  * v14,
>  

Re: [Qemu-devel] [PATCH v3] vfio/common: Check iova with limit not with size

[Pierre Morel]

On 01/20/2016 04:46 PM, Alex Williamson wrote:

On Wed, 2016-01-20 at 16:14 +0100, Pierre Morel wrote:

On 01/12/2016 07:16 PM, Alex Williamson wrote:

On Tue, 2016-01-12 at 16:11 +0100, Pierre Morel wrote:

In vfio_listener_region_add(), we try to validate that the region
is
not
zero sized and hasn't overflowed the addresses space.

But the calculation uses the size of the region instead of
using the region's limit (size - 1).

This leads to Int128 overflow when the region has
been initialized to UINT64_MAX because in this case
memory_region_init() transform the size from UINT64_MAX
to int128_2_64().

Let's really use the limit by sustracting one to the size
and take care to use the limit for functions using limit
and size to call functions which need size.

Signed-off-by: Pierre Morel 
---

Changes from v2:
  - all, just ignore v2, sorry about this,
this is build after v1

Changes from v1:
  - adjust the tests by knowing we already substracted one to
end.

   hw/vfio/common.c |   14 +++---
   1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 6797208..a5f6643 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -348,12 +348,12 @@ static void
vfio_listener_region_add(MemoryListener *listener,
   if (int128_ge(int128_make64(iova), llend)) {
   return;
   }
-end = int128_get64(llend);
+end = int128_get64(int128_sub(llend, int128_one()));
   
-if ((iova < container->min_iova) || ((end - 1) > container-

max_iova)) {

+if ((iova < container->min_iova) || (end  > container-

max_iova)) {

   error_report("vfio: IOMMU container %p can't map guest
IOVA
region"
" 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx,
- container, iova, end - 1);
+ container, iova, end);
   ret = -EFAULT;
   goto fail;
   }
@@ -363,7 +363,7 @@ static void
vfio_listener_region_add(MemoryListener *listener,
   if (memory_region_is_iommu(section->mr)) {
   VFIOGuestIOMMU *giommu;
   
-trace_vfio_listener_region_add_iommu(iova, end - 1);

+trace_vfio_listener_region_add_iommu(iova, end);
   /*
* FIXME: We should do some checking to see if the
* capabilities of the host VFIO IOMMU are adequate to
model
@@ -394,13 +394,13 @@ static void
vfio_listener_region_add(MemoryListener *listener,
   section->offset_within_region +
   (iova - section->offset_within_address_space);
   
-trace_vfio_listener_region_add_ram(iova, end - 1, vaddr);

+trace_vfio_listener_region_add_ram(iova, end, vaddr);
   
-ret = vfio_dma_map(container, iova, end - iova, vaddr,

section-

readonly);

+ret = vfio_dma_map(container, iova, end - iova + 1, vaddr,
section->readonly);
   if (ret) {
   error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx", %p) = %d (%m)",
- container, iova, end - iova, vaddr, ret);
+ container, iova, end - iova + 1, vaddr,
ret);
   goto fail;
   }
   

Hmm, did we just push the overflow from one place to another?  If
we're
mapping a full region of size int128_2_64() starting at iova zero,
then
this becomes (0x___ - 0 + 1) = 0.  So I think we
need
to calculate size with 128bit arithmetic too and let it assert if
we
overflow, ie:

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index a5f6643..13ad90b 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -321,7 +321,7 @@ static void
vfio_listener_region_add(MemoryListener *listener,
MemoryRegionSection
*section)
   {
   VFIOContainer *container = container_of(listener,
VFIOContainer, listener);
-hwaddr iova, end;
+hwaddr iova, end, size;
   Int128 llend;
   void *vaddr;
   int ret;
@@ -348,7 +348,9 @@ static void
vfio_listener_region_add(MemoryListener *listener,
   if (int128_ge(int128_make64(iova), llend)) {
   return;
   }
+
   end = int128_get64(int128_sub(llend, int128_one()));
+size = int128_get64(int128_sub(llend, int128_make64(iova)));

here again, if iova is null, since llend is section->size (2^64) ...

   
   if ((iova < container->min_iova) || (end  > container-

max_iova)) {

   error_report("vfio: IOMMU container %p can't map guest
IOVA region"
@@ -396,11 +398,11 @@ static void
vfio_listener_region_add(MemoryListener *listener,
   
   trace_vfio_listener_region_add_ram(iova, end, vaddr);
   
-ret = vfio_dma_map(container, iova, end - iova + 1, vaddr,

section->readonly);
+ret = vfio_dma_map(container, iova, size, vaddr, section-

readonly);

   if (ret) {
   error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx", %p) = %d (%m)",
- container, iova, end - iova + 1, vaddr, 

Re: [Qemu-devel] [PATCH v2 12/17] qcow2: convert QCow2 to use QCryptoBlock for encryption

[Fam Zheng]

On Thu, 01/21 10:50, Daniel P. Berrange wrote:
> On Thu, Jan 21, 2016 at 05:54:23PM +0800, Fam Zheng wrote:
> > On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > > This converts the qcow2 driver to make use of the QCryptoBlock
> > > APIs for encrypting image content. As well as continued support
> > > for the legacy QCow2 encryption format, the appealing benefit
> > > is that it enables support for the LUKS format inside qcow2.
> > 
> > FWIW, with today's QEMU, it's possible to stack format drivers on top of 
> > each
> > other.  In other words, even without this patch, we can make LUKS driver
> > encrypt/decrypt the qcow2 payload, while keeping them completely orthogonal.
> 
> Yep, that is certainly possible, and it is what is intended for using
> LUKS with RBD, iSCSI, & other network drivers.
> 
> I think there is value in having LUKS integrated directly into qcow2
> though. It means that given a qcow2 file you can 100% reliably
> distinguish between a file created with the intention of QEMU managing
> the LUKS encryption, from a file where the guest OS happens to have
> set up LUKS encryption in its virtual disk. If you don't have this,
> then given a random qcow2 file, you have to probe to see if LUKS is
> present or not. Given the security issues we've had in the past with
> raw images being turned into qcow2 images by a malicious guest writing
> a qcow2 header, I feel that having explicitly integration LUKS support
> in QCow is worthwhile as a concept.

Yes, I'm not objecting to explicit (read: mandatory) encryption in qcow2 in any
way, and extending the format spec for LUKS is a good thing.

I mentioned stacked drivers because I wonder how good we can do in reusing
block/crypto.c code to achieve this (to save 500+ new code in qcow2). For
example I have a rough idea along this:

* In qcow2 spec, define type "2" of crypt_method for LUKS encryption, and state
  that if this method is used, the payload must be LUKS format as defined in
  cryptsetup's docs/on-disk-format.pdf, and driver will take care of
  encrypting/decrypting data in a way that is transparent to guest.

* In qcow2_open, if header's crypt_method is LUKS, set a special flag in the
  BlockDriverState to indicate that block layer should create an overlay crypto
  driver (which will be luks in this case), on top of this BDS. To do this we
  need some modification to bdrv_open.

* When qcow2_open is done, block layer will then instantiate a LUKS driver,
  which backed by the qcow2 BDS (as the BDS.file). This LUKS BDS will then be
  attached to guest device.

* From the guest PoV, R/W reqs are served as if it's an ordinary qcow2; from
  LUKS driver's PoV, it works properly formatted luks data, which is backed by
  whatever it doesn't care, be it an iscsi target, rbd block, local file, or
  a qcow2 image that has metadata; from the qcow2 driver's PoV, it only made
  sure that a LUKS driver covers itself, at instantiation time.  Everything
  else operates the same way as a non-encrypted qcow2.

* Of course, qcow2_create would need some similar changes to the block layer as
  to bdrv_open, but that shouldn't be too hard.

Makes sense?

Fam

Re: [Qemu-devel] [PATCH v2 05/17] crypto: add support for anti-forensic split algorithm

[Fam Zheng]

On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> diff --git a/crypto/afsplit.c b/crypto/afsplit.c
> new file mode 100644
> index 000..42529e7
> --- /dev/null
> +++ b/crypto/afsplit.c
> @@ -0,0 +1,162 @@
> +/*
> + * QEMU Crypto anti forensic information splitter
> + *
> + * Copyright (c) 2015-2016 Red Hat, Inc.
> + *
> + * Derived from cryptsetup package lib/lusk1/af.c

s/lusk1/luks1/

> + *
> + * Copyright (C) 2004, Clemens Fruhwirth 
> + * Copyright (C) 2009-2012, Red Hat, Inc. All rights reserved.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + *
> + */
> +



> +
> +/**
> + * qcrypto_afsplit_encode:
> + * @hash: the hash algorithm to use for data expansion
> + * @blocklen: the size of @in in bytes
> + * @stripes: the number of times to expand @in in size
> + * @in: the master key to be expanded in size
> + * @out: preallocted buffer to hold the split key
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Split the data in @in, which is @blocklen bytes in
> + * size, to form a larger piece of data @out, which is
> + * @blocklen * @stripes bytes in size.
> + *
> + * Returns: 0 on success, -1 on error;
> + */
> +int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
> +   size_t blocklen,
> +   uint32_t stripes,
> +   const uint8_t *in,
> +   uint8_t *out,
> +   Error **errp);
> +
> +/**
> + * qcrypto_afsplit_decode:
> + * @hash: the hash algorithm to use for data compression
> + * @blocklen: the size of @out in bytes
> + * @stripes: the number of times to decrease @in in size
> + * @in: the master key to be expanded in size
> + * @out: preallocted buffer to hold the split key

I think the descriptions for @in and @out are wrong.

> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Join the data in @in, which is @blocklen * @stripes
> + * bytes in size, to form the original small piece o

piece of

> + * data @out, which is @blocklen bytes in size.
> + *
> + * Returns: 0 on success, -1 on error;
> + */
> +int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
> +   size_t blocklen,
> +   uint32_t stripes,
> +   const uint8_t *in,
> +   uint8_t *out,
> +   Error **errp);
> +
> +#endif /* QCRYPTO_AFSPLIT_H__ */

Fam

Re: [Qemu-devel] [PATCH v9 10/37] qapi: Make all visitors supply uint64 callbacks

[Markus Armbruster]

Eric Blake  writes:

> On 01/20/2016 10:29 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> Our qapi visitor contract supports multiple integer visitors,
>>> but left the type_uint64 visitor as optional (falling back on
>>> type_int64); it also marks the type_size visitor as optional
>>> (falling back on type_uint64 or even type_int64).
>>>
>>> Note that the default of falling back on type_int for unsigned
>>> visitors can cause confusing results for values larger than
>>> INT64_MAX (such as having to pass in a negative 2s complement
>>> value on input, and getting a negative result on output).
>>>
>>> This patch does not fully address the disparity in handling
>>> large values as negatives, but does move towards a cleaner
>>> situation where EVERY visitor provides both type_int64 and
>>> type_uint64 variants as entry points; then each client can
>>> either implement sane differences between the two, or document
>>> in place with a FIXME that there is munging going on.
>> 
>> Before: nobody implements type_uint64(), and the core falls back to
>> type_int64(), casting negative values to large positive ones.  With an
>> implementation of type_int64() that parses large positive values as
>> negative, the two casts cancel out.
>> 
>> After: everybody implements type_uint64() incorrectly, by reusing
>> type_int64() code.  The problem moves from visitor core to visitor
>> implementations, where we can actually fix it if we care.
>> 
>> Correct?
>
> Close. opts-visitor.c already implemented type_uint64, but also has its
> known bugs (and Paolo has been pinged about his claim for fixes in that
> file...)

Ah, yes.  opts_type_uint64() doesn't reuse opts_type_int64(), but
largely duplicates it.  Potentially less wrong :)

> But otherwise, yes, in this patch, we are not fixing insanity so much as
> localizing and better documenting it.

Let me try to clarify the commit message a bit:

This patch does not address the disparity in handling large values
as negatives.  It merely moves the fallback from uint64 to int64
from the visitor core to the visitors, where the issue can actually
be fixed, by implementing the missing type_uint64() callbacks on top
of the respective type_int64() callbacks, with a FIXME comment
explaining why that's wrong.

> I've given some thought about converting the QObject 'int' type to track
> a sign bit, making it effectively a superset covering 3*2^63 values in
> the range [INT64_MIN, UINT64_MAX], and then enhancing the parser to
> track if a negative value was parsed and the formatter to output
> negative if the sign bit is set, and then make the 'int64' and 'uint64'
> parsers stick to stricter 2*64 subsets of that range.  But I haven't
> actually written a patch along those lines yet.

Yes, we'll want to get that right eventually, but it's not urgent.

>>> The dealloc visitor no longer needs a type_size callback,
>>> since that now safely falls back to the type_uint64 callback.
>> 
>> Did it need the callback before this patch?
>
> Not really.  Should I split out the deletion of that callback as a
> separate patch?

Probably cleanest, but merely adjusting the commit message would also
work for me:

The dealloc visitor doesn't actually need a type_size() callback,
since the visitor core can safely fall back to the type_uint64()
callback.  Drop it.

>>> Then, in qapi-visit-core.c, we can now use the guaranteed
>>> type_uint64 callback as the fallback for all smaller unsigned
>>> int visits.
>> 
>> The type_int64() callback works just fine for smaller unsigned ints, but
>> I agree avoiding mixed signedness by using type_uint64() make sense once
>> type_uint64() is mandatory.
>> 
>
>>> +++ b/qapi/qapi-visit-core.c
>>> @@ -102,15 +102,15 @@ void visit_type_int(Visitor *v, int64_t *obj, const 
>>> char *name, Error **errp)
>>>
>>>  void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error 
>>> **errp)
>>>  {
>>> -int64_t value;
>>> +uint64_t value;
>>>
>>>  if (v->type_uint8) {
>>>  v->type_uint8(v, obj, name, errp);
>>>  } else {
>>>  value = *obj;
>>> -v->type_int64(v, , name, errp);
>>> -if (value < 0 || value > UINT8_MAX) {
>>> -/* FIXME questionable reuse of errp if type_int64() changes
>>> +v->type_uint64(v, , name, errp);
>>> +if (value > UINT8_MAX) {
>>> +/* FIXME questionable reuse of errp if type_uint64() changes
>>> value on error */
>>>  error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
>>> name ? name : "null", "uint8_t");
>> 
>> You could delay adding these FIXMEs until this patch, and reduce churn.
>> Probably not worth the bother now.
>
> Yeah, I'll see how the rest of the series review goes.

Hope to make some more progress today.

Re: [Qemu-devel] [PULL 14/15] qemu-char: add logfile facility to all chardev backends

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote:
> Hi,
> 
> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the 
> following command line:
> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0
> 
> Before:
> [nothing is print on console]
> 
> After:
> QEMU 2.5.50 monitor - type 'help' for more information
> (qemu)
> 
> In both cases, a working vc is created in the GTK+ UI.
> 
> Reverting the commit (and fixing the trivial conflict) makes things
> work again.

Thanks for the heads-up, I'll investigate and submit a fix


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v9 12/37] qapi: Don't cast Enum* to int*

[Markus Armbruster]

Eric Blake  writes:

> On 01/20/2016 11:08 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> C compilers are allowed to represent enums as a smaller type
>>> than int, if all enum values fit in the smaller type.  There
>>> are even compiler flags that force the use of this smaller
>>> representation, and using them changes the ABI of a binary.
>> 
>> Suggest "although using them".
>> 
>
> Okay.
>
>
>>> with generated code changing as:
>>>
>>> | void visit_type_GuestDiskBusType(Visitor *v, GuestDiskBusType
>>> | *obj, const char *name, Error **errp)
>>> | {
>>> |-visit_type_enum(v, (int *)obj, GuestDiskBusType_lookup, 
>>> "GuestDiskBusType", name, errp);
>>> |+int tmp = *obj;
>>> |+visit_type_enum(v, , GuestDiskBusType_lookup, "GuestDiskBusType", 
>>> name, errp);
>>> |+*obj = tmp;
>>> | }
>> 
>> Long lines.  Do we have an example with a shorter enum name handy?
>
> Shortest is QType; runner-ups RxState and TpmType.

QType comes out okay:

| void visit_type_QType(Visitor *v, QType *obj, const char *name, Error **errp)
| {
|-visit_type_enum(v, (int *)obj, QType_lookup, "QType", name, errp);
|+int tmp = *obj;
|+ visit_type_enum(v, , QType_lookup, "QType", name, errp);
|+*obj = tmp;
| }

Let's use it.

>>>  void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, 
>>> Error **errp)
>>>  {
>>> -visit_type_enum(v, (int *)obj, %(c_name)s_lookup, "%(name)s", name, 
>>> errp);
>>> +int tmp = *obj;
>>> +visit_type_enum(v, , %(c_name)s_lookup, "%(name)s", name, errp);
>>> +*obj = tmp;
>>>  }
>>>  ''',
>>>   c_name=c_name(name), name=name)
>> 
>> Same pattern in qapi-visit-core.c, except we name the variable @value
>> there.  Your choice.
>
> 'value' sounds consistent. An easy swap on a respin.

Okay.

Re: [Qemu-devel] [PATCH v5 1/4] Provide support for the CUSE TPM

[Michael S. Tsirkin]

On Thu, Jan 21, 2016 at 05:41:32AM +, Xu, Quan wrote:
> > On January 21, 2016 at 1:08pm,  wrote:
> > On Wed, Jan 20, 2016 at 04:25:15PM -0500, Stefan Berger wrote:
> > > On 01/20/2016 01:54 PM, Michael S. Tsirkin wrote:
> > > >On Wed, Jan 20, 2016 at 11:06:45AM -0500, Stefan Berger wrote:
> > > >>"Michael S. Tsirkin"  wrote on 01/20/2016 10:58:02 AM:
> > > >>
> > > >>>From: "Michael S. Tsirkin"  On Wed, Jan 20, 2016 at
> > > >>>10:36:41AM -0500, Stefan Berger wrote:
> > > "Michael S. Tsirkin"  wrote on 01/20/2016 10:20:58
> > AM:
> > > 
> > > >From: "Michael S. Tsirkin" 
> > > >>The CUSE TPM and associated tools can be found here:
> > > >>
> > > >>https://github.com/stefanberger/swtpm
> > > >>
> > > >>(please use the latest version)
> > > >>
> > > >>To use the external CUSE TPM, the CUSE TPM should be started as
> > > >>follows:
> > > >># terminate previously started CUSE TPM /usr/bin/swtpm_ioctl -s
> > > >>/dev/vtpm-test
> > > >>
> > > >># start CUSE TPM
> > > >>/usr/bin/swtpm_cuse -n vtpm-test
> > > >>
> > > >>QEMU can then be started using the following parameters:
> > > >>
> > > >>qemu-system-x86_64 \
> > > >>[...] \
> > > >> -tpmdev cuse-tpm,id=tpm0,cancel-path=/dev/null,path=/
> > > >>>dev/vtpm-test
> > > \
> > > >> -device tpm-tis,id=tpm0,tpmdev=tpm0 \
> > > >>[...]
> > > >>
> > > >>
> > > >>Signed-off-by: Stefan Berger 
> > > >>Cc: Eric Blake 
> > > >Before we add a dependency on this interface, I'd rather see this
> > > >interface supported in kernel and not just in CUSE.
> > > For using the single hardware TPM, we have the passthrough type.
> > > >>>It's usage is
> > > limited.
> > > 
> > > CUSE extends the TPM character device interface with ioctl's.
> > > Behind the character device we can implement a TPM 1.2 and a TPM
> > > 2. Both TPM implementations require large amounts of code, which I
> > > don't thinkshould go into the Linux kernel itself. So I don't know
> > > who would implement this interface inside the Linux kernel.
> > > 
> > >    Stefan
> > > 
> > > >>>BTW I'm not talking about the code - I'm talking about the interfaces 
> > > >>>here.
> > > >>>
> > > >>>One way would be to add support for these interface support in the 
> > > >>>kernel.
> > > >>>
> > > >>>Maybe others can be replaced with QMP events so management can take
> > > >>>the necessary action.
> > > >>>
> > > >>>As long as this is not the case, I suspect this code will have to
> > > >>>stay out of tree :( We can't depend on interfaces provided solely
> > > >>>by cuse devices on github.
> > > >>Why is that? I know that the existing ioctls cannot be modified
> > > >>anymore once QEMU accepts the code. So I don't understand it. Some
> > > >>of the ioctls are only useful when emulating a hardware device,
> > > >Maybe they can be replaced with QMP events?
> > > >These could be emitted unconditionally, and ignored by management in
> > > >passthrough case.
> > > >
> > > >>so there's no need for them in a
> > > >>kernel interface unless one was to put the vTPM code into the Linux
> > > >>kernel, but I don't see that this is happening. What is better about
> > > >>a kernel interface versus one implemented by a project on github
> > > >>assuming that the existing ioctls are stable? What is the real reason 
> > > >>here?
> > > >>
> > > >>Stefan
> > > >>
> > > >That someone went to the trouble of reviewing the interface for
> > > >long-term maintainability, portability etc. That it obeys some
> > > >existing standards for API use, coding style etc and will continue to.
> > >
> > > The same applies to the libtpms and swtpm projects as well, I suppose.
> > > If someone wants to join them, let me know.
> > >
> > > As stated, we will keep the existing ioctl stables once integrated but
> > > will adapt where necessary before that.
> > > >
> > > >In other words, kernel is already a dependency for QEMU.
> > >
> > > I don't see vTPM going into the kernel, at least I don't know of
> > > anyone trying to do that.
> > >
> > >Stefan
> > >
> > 
> > Well that was just one idea, it's up to you guys.
> > But while modular multi-process QEMU for security might happen in future, I
> > don't see us doing this by moving large parts of QEMU into cuse devices, and
> > talking to these through ioctls.
> 
> 
> IIUC, the major root issue is that CUSE TPM is based on soft defined TPM, 
> instead of hardware TPM.

No, the major issue is in how it's put together.

> This may bring in more security/stability issues.. 
> As I know, some trusted cloud products must base on upstream code. TPM 
> passthru is just for limited VM.
> As I mentioned, I think CUSE TPM is a good solution for trusted cloud.
>
> Hagen, could you share more user 

Re: [Qemu-devel] [PATCH v9 15/37] qom: Swap 'name' next to visitor in ObjectPropertyAccessor

[Markus Armbruster]

Eric Blake  writes:

> On 01/20/2016 11:49 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> Similar to the previous patch, it's nice to have all functions
>>> in the tree that involve a visitor and a name for conversion to
>>> or from QAPI to consistently stick the 'name' parameter next
>>> to the Visitor parameter.
>>>
>>> Done by manually changing include/qom/object.h and qom/object.c,
>>> then running this Coccinelle script and touching up the fallout
>>> (Coccinelle insisted on adding some trailing whitespace).
>>>
>>> @ rule1 @
>>> identifier fn;
>>> type Object, Visitor, Error;
>>> identifier obj, v, opaque, name, errp;
>>> @@
>>>  void fn
>>> - (Object *obj, Visitor *v, void *opaque, const char *name,
>>> + (Object *obj, Visitor *v, const char *name, void *opaque,
>>>Error **errp) { ... }
>> 
>> I think we want to match void functions with exactly these parameter
>> types.  The parameter names don't matter.
>
> The parameter names shouldn't matter; the 'identifier obj' should have
> been enough to make 'obj' a metavariable matching any actual parameter name.
>
>> 
>> However, the actual match is looser!  For instance, it also matches
>> 
>> void foo(int *pi, unsigned *pu, void *vp, const char *cp, double **dpp)
>> {
>> }
>
> Uggh. My intent was to match exactly 'Object *' and 'Visitor *' as the
> first two types, where 'int *' and 'unsigned *' are NOT matches.  But I
> don't know Coccinelle well enough to make that blatantly obvious (is my
> declaration of 'type Object' not correct?).

'type Object' makes 'Object' a metavariable matching any C type.

>> This could mess up unrelated function.  I could double-check it doesn't,
>> but I'd rather have a narrower match instead.  Can't give one offhand,
>> though.  Ideas?
>
> Is 'typedef' better than 'type' for constraining the type of the first
> two arguments?

Matches any C typedef name.  Less wrong, but still wrong :)

> Or does Coccinelle do literal matches on anything you
> don't pre-declare, as in:

Yes, but...

>  @ rule1 @
>  identifier fn;
>  identifier obj, v, opaque, name, errp;
>  @@
>   void fn
>  - (Object *obj, Visitor *v, void *opaque, const char *name,
>  + (Object *obj, Visitor *v, const char *name, void *opaque,
> Error **errp) { ... }

... when I try that, spatch throws a parse error.

> Fortunately, a manual inspection of the results (which I had to do
> anyways due to spacing issues) didn't spot any incorrect swaps.
>
> At this point, I don't know that re-writing Coccinelle will be worth the
> hassle (nothing else needs to be rewritten).

I'd normally take up the challenge to wrestle Coccinelle, but I think
our time is better spent elsewhere right now.  Let's amend the commit
message instead: describe the semantic patch's shortcomings, and your
manual checking:

This semantic patch is actually far too lose: it matches *any* type,
not just Object, Visitor, Error.  However, I can't figure out how to
make Coccinelle match Object, Visitor, Error exactly.  Simply
deleting the 'type Object, Visitor, Error;' line results in a parse
error.  I reviewed the resulting patch carefully to verify there are
no unwanted matches.

>>> @@
>>> identifier rule1.fn;
>>> expression obj, v, opaque, name, errp;
>>> @@
>>>  fn(obj, v,
>>> -   opaque, name,
>>> +   name, opaque,
>>> errp)
>> 
>> The rule1.fn restricts the match to functions changed by the previous
>> rule.  Good.
>> 
>>>
>>> Signed-off-by: Eric Blake 
>>> Reviewed-by: Marc-André Lureau 
>> 

Re: [Qemu-devel] [PATCH v9 16/37] qapi: Swap 'name' in visit_* callbacks to match public API

[Markus Armbruster]

Eric Blake  writes:

> On 01/20/2016 11:55 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> As explained in the previous patches, matching argument order of
>>> 'name, ' to JSON's "name":value makes sense.  However,
>>> while the last two patches were easy with Coccinelle, I ended up
>>> doing this one all by hand.  Now all the visitor callbacks match
>>> the main interface.
>>>
>>> Signed-off-by: Eric Blake 
>>> Reviewed-by: Marc-André Lureau 
>>>
>
>>> @@ -30,39 +30,42 @@ struct Visitor
>>>  GenericList *(*next_list)(Visitor *v, GenericList **list, Error 
>>> **errp);
>>>  void (*end_list)(Visitor *v, Error **errp);
>>>
>>> -void (*type_enum)(Visitor *v, int *obj, const char * const strings[],
>>> -  const char *kind, const char *name, Error **errp);
>>> +void (*type_enum)(Visitor *v, const char *name, int *obj,
>>> +  const char * const strings[], const char *kind,
>> 
>> Opportunity to change to 'const char *const'.  I prefer that, because it
>> makes the fact that this is a pointer-* and not a binary operator-*
>> visually obvious.
>> 
>> Same elsewhere.
>
> Hmm, I probably have churn later in the series.  Will fix.
>
>>>  /* May be NULL; most useful for input visitors. */
>>> -void (*optional)(Visitor *v, bool *present, const char *name);
>>> +void (*optional)(Visitor *v, const char *name, bool *present);
>>>
>
>> 
>> I checked the changes to this file carefully.  Can we rely on the
>> compiler to flag mistakes in the rest of the patch?
>
> C's (intentionally-loose) treatment of 'char *' like 'void *' is a bit
> worrisome, but the fact that we have 'const' on only one of the two
> swapped arguments was indeed enough to make the compiler complain about
> mismatch in parameter types when trying to assign incorrectly-typed
> static functions to the updated struct members.

Okay, I guess that's good enough.

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Paolo Bonzini]

On 21/01/2016 10:52, Gerd Hoffmann wrote:
>> > Instead of having a full-blown thread, are there things (such as the
>> > TGSI->GLSL conversion) that could be simply offloaded to a userspace
>> > thread pool, either in QEMU or in virglrenderer?
> I think virglrenderer would have to do that.  Unfortunaly opengl isn't
> very good at multithreading, so I'm not sure a thread pool would work
> well.  Compiling shaders could be a special case where threading
> actually works because that isn't in the actual rendering workflow.  Not
> fully sure though, Dave?

Or even MESA could. Perhaps we (you :)) could add an extension for
asynchronous shader compilation.  But the TGSI->GLSL conversion is done
by virglrender, with no opengl calls (with a big IIRC in front).

Paolo

Re: [Qemu-devel] [PATCH v2 10/17] block: add generic full disk encryption driver

[Fam Zheng]

On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> +/* XXX Should we treat size as being total physical size
> + * of the image (ie payload + encryption header), or just
> + * the logical size of the image (ie payload). If the latter
> + * then we need to extend 'size' to include the header
> + * size */

The latter. :)

> +qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, _abort);
> +#define BLOCK_CRYPTO_DRIVER(name, format)   \
> +static int block_crypto_probe_ ## name(const uint8_t *buf,  \
> +   int buf_size,\
> +   const char *filename) {  \
> +return block_crypto_probe_generic(format,   \
> +  buf, buf_size, filename); \
> +}   \
> +\
> +static int block_crypto_open_ ## name(BlockDriverState *bs, \
> +  QDict *options,   \
> +  int flags,\
> +  Error **errp) \
> +{   \
> +return block_crypto_open_generic(format,\
> + _crypto_runtime_opts_ ## 
> name, \
> + bs, options, flags, errp); \
> +}   \
> +\
> +static int block_crypto_create_ ## name(const char *filename,   \
> +QemuOpts *opts, \
> +Error **errp)   \
> +{   \
> +return block_crypto_create_generic(format,  \
> +   filename, opts, errp);   \
> +}   \
> +\
> +BlockDriver bdrv_crypto_ ## name = {\
> +.format_name= #name,\
> +.instance_size  = sizeof(BlockCrypto),  \
> +.bdrv_probe = block_crypto_probe_ ## name,  \
> +.bdrv_open  = block_crypto_open_ ## name,   \
> +.bdrv_close = block_crypto_close,   \
> +.bdrv_create= block_crypto_create_ ## name, \
> +.create_opts= _crypto_create_opts_ ## name,   \
> +\
> +.bdrv_co_readv  = block_crypto_co_readv,\
> +.bdrv_co_writev = block_crypto_co_writev,   \
> +.bdrv_getlength = block_crypto_getlength,   \
> +}
> +
> +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS);

Personally I really prefer a preprocessed version, for the ease of grep.

Fam

Re: [Qemu-devel] [PATCH v7] spec: add qcow2 bitmaps extension specification

[Kevin Wolf]

Am 21.01.2016 um 09:22 hat Vladimir Sementsov-Ogievskiy geschrieben:
> On 21.01.2016 00:22, John Snow wrote:
> >
> >On 01/20/2016 07:34 AM, Vladimir Sementsov-Ogievskiy wrote:
> >>On 19.01.2016 20:48, Kevin Wolf wrote:
> >>>Am 11.01.2016 um 14:05 hat Vladimir Sementsov-Ogievskiy geschrieben:
> The new feature for qcow2: storing bitmaps.
> 
> This patch adds new header extension to qcow2 - Bitmaps Extension. It
> provides an ability to store virtual disk related bitmaps in a qcow2
> image. For now there is only one type of such bitmaps: Dirty Tracking
> Bitmap, which just tracks virtual disk changes from some moment.
> 
> Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
> should be stored in this qcow2 file. The size of each bitmap
> (considering its granularity) is equal to virtual disk size.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> v7:
> 
> - Rewordings, grammar.
> Max, Eric, John, thank you very much.
> 
> - add last paragraph: remaining bits in bitmap data clusters must be
> zero.
> 
> - s/Bitmap Directory/bitmap directory/ and other names like this at
> the request of Max.
> 
> v6:
> 
> - reword bitmap_directory_size description
> - bitmap type: make 0 reserved
> - extra_data_size: resize to 4bytes
> Also, I've marked this field as "must be zero". We can always change
> it, if we decide allowing managing app to specify any extra data, by
> defining some magic value as a top of user extra data.. So, for now
> non zeor extra_data_size should be considered as an error.
> - swap name and extra_data to give good alignment to extra_data.
> 
> 
> v5:
> 
> - 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
> bitmaps.
> - rewordings
> - move upper bounds to "Notes about Qemu limits"
> - s/should/must somewhere. (but not everywhere)
> - move name_size field closer to name itself in bitmap header
> - add extra data area to bitmap header
> - move bitmap data description to separate section
> 
>    docs/specs/qcow2.txt | 172
> ++-
>    1 file changed, 171 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 121dfc8..997239d 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -103,7 +103,18 @@ in the description of a field.
>    write to an image with unknown auto-clear
> features if it
>    clears the respective bits from this field first.
>    -Bits 0-63:  Reserved (set to 0)
> +Bit 0:  Bitmaps extension bit
> +This bit indicates consistency for
> the bitmaps
> +extension data.
> +
> +It is an error if this bit is set
> without the
> +bitmaps extension present.
> +
> +If the bitmaps extension is present
> but this
> +bit is unset, the bitmaps extension
> data is
> +inconsistent.
> >>>It may as well be consistent, but we don't know.
> >>>
> >>>Perhaps something like "must be considered inconsistent" or "is
> >>>potentially inconsistent".
> >>>
> +
> +Bits 1-63:  Reserved (set to 0)
>   96 -  99:  refcount_order
>    Describes the width of a reference count block
> entry (width
> @@ -123,6 +134,7 @@ be stored. Each extension has a structure like
> the following:
>    0x - End of the header extension area
>    0xE2792ACA - Backing file format name
>    0x6803f857 - Feature name table
> +0x23852875 - Bitmaps extension
>    other  - Unknown header extension, can
> be safely
> ignored
>    @@ -166,6 +178,34 @@ the header extension data. Each entry look
> like this:
>    terminated if it has full length)
>  +== Bitmaps extension ==
> +
> +The bitmaps extension is an optional header extension. It provides
> the ability
> +to store bitmaps related to a virtual disk. For now, there is only
> one bitmap
> +type: the dirty tracking bitmap, which tracks virtual disk changes
> from some
> +point in time.
> >>>I have one major problem with this patch, and it starts here.
> >>>
> >>>The spec talks about dirty tracking bitmaps all the way, but it never
> 

Re: [Qemu-devel] [PATCH v2 00/19] ISA DMA controllers cleanup (i8257, i82374)

[Paolo Bonzini]

On 20/01/2016 21:31, Hervé Poussineau wrote:
> Ping.
> 
> Hervé
> 
> Le 10/01/2016 16:24, Hervé Poussineau a écrit :
>> Hi,
>>
>> This patchset is a cleanup of the i8257/i82374 ISA DMA controllers.
>> Global DMA_* functions will be obsoleted and then deleted, and ISA
>> devices will not
>> be tied anymore to i8257 DMA device implementation.
>>
>> This paves the way to fix support for floppy DMA operations on
>> sparc/sparc64/MIPS Magnum
>> platforms (which don't use a i8257 DMA controller), and to support
>> multiple ISA buses on
>> the same machine.
>>
>> Patch 1 cleans up the i82374 DMA controller, by removing device
>> inheritance.
>> Patches 2 to 9 change i8257 to current standards (structures and
>> functions renaming, QOM)
>> Patches 10 to 18 create and use a IsaDma interface, to separate
>> devices from i8257
>> device implementation.
>> Patch 19 removes now unused DMA_* functions.
>>
>> Hervé
>>
>> Changes since v1:
>> - added patches 4 and 9
>> - simplify patch 12, so that it compiles alone (John Snow)
>> - fix warning with clang (John Snow)

FWIW, the general idea looks great to me.  If John is okay, just send a
pull request yourself.

Paolo

>> Hervé Poussineau (19):
>>i82374: device only existed as ISA device, so simplify device
>>i8257: pass ISA bus to DMA_init() function
>>i8257: rename struct dma_cont to I8257State
>>i8257: rename struct dma_regs to I8257Regs
>>i8257: rename functions to start with i8257_ prefix
>>i8257: make the DMA running method per controller
>>i8257: add missing const
>>i8257: QOM'ify
>>i8257: move state definition to new independent header
>>isa: add an ISA DMA interface, and store it within the ISA bus
>>i8257: implement the IsaDma interface
>>magnum: disable floppy DMA for now
>>sparc: disable floppy DMA
>>sparc64: disable floppy DMA
>>fdc: use IsaDma interface instead of global DMA_* functions
>>cs4231a: use IsaDma interface instead of global DMA_* functions
>>gus: use IsaDma interface instead of global DMA_* functions
>>sb16: use IsaDma interface instead of global DMA_* functions
>>dma: remove now useless DMA_* functions
>>
>>   hw/audio/cs4231a.c  |  23 ++-
>>   hw/audio/gus.c  |  20 ++-
>>   hw/audio/sb16.c |  23 ++-
>>   hw/block/fdc.c  |  65 +---
>>   hw/dma/i82374.c |  58 +++
>>   hw/dma/i8257.c  | 395
>> ++--
>>   hw/i386/pc.c|   2 +-
>>   hw/isa/isa-bus.c|  21 +++
>>   hw/mips/mips_fulong2e.c |   2 +-
>>   hw/mips/mips_jazz.c |   5 +-
>>   hw/mips/mips_malta.c|   2 +-
>>   hw/sparc/sun4m.c|  24 +--
>>   hw/sparc64/sun4u.c  |  37 ++---
>>   include/hw/isa/i8257.h  |  42 +
>>   include/hw/isa/isa.h|  51 +--
>>   include/qemu/typedefs.h |   1 +
>>   16 files changed, 454 insertions(+), 317 deletions(-)
>>   create mode 100644 include/hw/isa/i8257.h
>>
> 

Re: [Qemu-devel] [PATCH v2 00/13] Dirty bitmap changes for migration/persistence work

[Vladimir Sementsov-Ogievskiy]

An idea/question.

What about make some const dirty bitmap finctions really const?

bdrv_dirty_bitmap_name(BdrvDirtyBitmap *bitmap)   -> 
bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
bdrv_dirty_bitmap_size(BdrvDirtyBitmap *bitmap) -> 
bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)


etc..

I don't know, may be it is not Qemu style or there are some other 
restrictions, but in common case strict definitions are better.. In my 
persistent series I've included a fix for this "[PATCH 01/17] block: fix 
bdrv_dirty_bitmap_granularity()", necessary for my other functions with 
const BdrvDirtyBitmap *bitmap.


I can add such fixes into my series (but these series are better place, 
I think), or, if it is bad for Qemu, I'll remove all such const's from 
my series.



On 20.01.2016 09:11, Fam Zheng wrote:

v2: Various changes addressing John's and Vladimir's comments:

 [02/13] typedefs: Add BdrvDirtyBitmap
 Skip HBitmapIter because we'll hide it soon. [John]
 
 [03/13] block: Move block dirty bitmap code to separate files

 [04/13] block: Remove unused typedef of BlockDriverDirtyHandler
 [05/13] block: Hide HBitmap in block dirty bitmap interface
 Add assert in bdrv_dirty_bitmap_truncate(). [John]
 Add John's rev-by.

 [06/13] HBitmap: Introduce "meta" bitmap to track bit changes
 Caller of hbitmap_create_meta() frees it with hbitmap_free_meta().
 [John, Vladimir]

 [07/13] tests: Add test code for meta bitmap
 Add John's rev-by.

 [08/13] block: Support meta dirty bitmap
 Release the meta dirty bitmap in bdrv_release_dirty_bitmap().

 [09/13] block: Add two dirty bitmap getters
 [10/13] block: Assert that bdrv_release_dirty_bitmap succeeded
 Add John's rev-by.

 [11/13] hbitmap: serialization
 Fix comment for hbitmap_serialization_granularity() and
 hbitmap_deserialize_part(). [John]
 Document @finish in hbitmap_deserialize_zeroes more clearly.
 Fixed granularity in hbitmap_serialization_granularity().
 [Vladimir]
 Tweak the assertion in serialization_chunk. [Vladimir]
 cpu_to_leXXs -> leXX_to_cpus in hbitmap_deserialize_part.
 [Vladimir]
 Fix typo in serialization_chunk() comments. [John]

 [12/13] block: BdrvDirtyBitmap serialization interface
 [13/13] tests: Add test code for hbitmap serialization


Two major features are added to block dirty bitmap (and underlying HBitmap) in
this series: meta bitmap and serialization, together with all other supportive
patches.

Both operations are common in dirty bitmap migration and persistence: they need
to find whether and which part of the dirty bitmap in question has changed with
meta dirty bitmap, and they need to write it to the target with serialization.


Fam Zheng (11):
   backup: Use Bitmap to replace "s->bitmap"
   typedefs: Add BdrvDirtyBitmap
   block: Move block dirty bitmap code to separate files
   block: Remove unused typedef of BlockDriverDirtyHandler
   block: Hide HBitmap in block dirty bitmap interface
   HBitmap: Introduce "meta" bitmap to track bit changes
   tests: Add test code for meta bitmap
   block: Support meta dirty bitmap
   block: Add two dirty bitmap getters
   block: Assert that bdrv_release_dirty_bitmap succeeded
   tests: Add test code for hbitmap serialization

Vladimir Sementsov-Ogievskiy (2):
   hbitmap: serialization
   block: BdrvDirtyBitmap serialization interface

  block.c  | 339 -
  block/Makefile.objs  |   2 +-
  block/backup.c   |  25 ++-
  block/dirty-bitmap.c | 492 +++
  block/mirror.c   |  14 +-
  include/block/block.h|  40 +---
  include/block/dirty-bitmap.h |  71 +++
  include/qemu/hbitmap.h   |  95 +
  include/qemu/typedefs.h  |   2 +
  tests/test-hbitmap.c | 255 ++
  util/hbitmap.c   | 201 --
  11 files changed, 1126 insertions(+), 410 deletions(-)
  create mode 100644 block/dirty-bitmap.c
  create mode 100644 include/block/dirty-bitmap.h




--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] qxl : no mouse cursor with sdl2/gtk UI

[Gerd Hoffmann]

On Mi, 2016-01-20 at 15:20 +0100, nicolas prochazka wrote:
> hello, 
> 
> my libvirt definition xml file, if i change qxl by vga , mouse pointer
> is present , 
> 
> if i change vm seven to windows81 , mouse pointer is present  ( using
> wddm qxl driver not the same as seven )
> 
> with windows seven vm, last qxl , mouse click is ok, but there'is no
> mouse pointer
> 
> ( sound card, usb are passthrough device , test without passthrough do
> not change anyting ) 

What do you use usb passthrough for?  The mouse?

cheers,
  Gerd

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Dave Airlie]

On 18 January 2016 at 19:54, Gerd Hoffmann  wrote:
>   Hi folks,
>
> I'm starting to investigate if and how we can move the user interface
> code into its own thread instead of running it in the iothread and
> therefore avoid blocking the guest in case some UI actions take a little
> longer.
>
> opengl and toolkits tend to be bad at multithreading.  So my idea is to
> have a single thread dedicated to all the UI + rendering stuff, possibly
> let even the virglrenderer run in ui thread context.

I'd still like virgl to run it its own thread at some point like
dataplane but for virgl I suppose.

We see cases where backend GLSL compiles and end up taking quite a
while, and I'd prefer
to not block the UI or qemu on those.

I've hacked on this before, but only with SDL and it was pretty dirty,
and it gave a fairly decent
speed up.

My thoughts are to use dataplane like design to process the queue in a
separate thread,
and then have some sort of channel between the UI and virtio-gpu
thread to handle things like
screen resize etc.

http://cgit.freedesktop.org/~airlied/qemu/commit/?h=dave3d=fe22a0955255afef12b947c4a91efa57e7a7c429
is my previous horror patch.

Dave.

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Paolo Bonzini]

On 21/01/2016 09:44, Dave Airlie wrote:
> I've hacked on this before, but only with SDL and it was pretty dirty,
> and it gave a fairly decent
> speed up.
> 
> My thoughts are to use dataplane like design to process the queue in a
> separate thread,
> and then have some sort of channel between the UI and virtio-gpu
> thread to handle things like
> screen resize etc.
> 
> http://cgit.freedesktop.org/~airlied/qemu/commit/?h=dave3d=fe22a0955255afef12b947c4a91efa57e7a7c429
> is my previous horror patch.

Instead of having a full-blown thread, are there things (such as the
TGSI->GLSL conversion) that could be simply offloaded to a userspace
thread pool, either in QEMU or in virglrenderer?

If you can do this in QEMU, as an alternative to writing code with
callbacks we have coroutines based on a setjmp-longjmp and also a simple
thread pool (thread-pool.c).  If you want to allocate your own
thread-pool, you can create an additional AioContext (aio_context_new)
and add the AioContext to the glib main loop using aio_get_g_source.

Paolo

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Gerd Hoffmann]

  Hi,

> What are taking long time? Can you give a few examples?

Didn't profile things, but UI code is handling bulky graphics data,
which of course takes time, especially in case some software scaling is
involved.

For sdl-1 I remember complains about it slowing down guests alot.  IIRC
I checked a while back and figured it does some X11 stuff synchronously
(i.e. waits on X server reply) somewhere deep in the library.

With virglrenderer a new issue came up: compiling shaders can take time.

Given both opengl and ui toolkits tend to be pretty bad at
multithreading I'm not sure how well helper threads for heavier UI tasks
are going to work (I suspect that is the background of your question).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 12/17] qcow2: convert QCow2 to use QCryptoBlock for encryption

[Fam Zheng]

On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> This converts the qcow2 driver to make use of the QCryptoBlock
> APIs for encrypting image content. As well as continued support
> for the legacy QCow2 encryption format, the appealing benefit
> is that it enables support for the LUKS format inside qcow2.

FWIW, with today's QEMU, it's possible to stack format drivers on top of each
other.  In other words, even without this patch, we can make LUKS driver
encrypt/decrypt the qcow2 payload, while keeping them completely orthogonal.

It's someting like:

   
   |   LUKS   |
   
|
v
   
   |  qcow2   |
   
|
v
   
   |   file   |
   

The command line looks like this:

 -drive driver=luks,file.driver=qcow2,file.file.driver=file,\
file.file.filename=$qcow2_image_whose_payload_is_in_luks_format

unfortunately I don't know how to create nested images with qemu-img. I tested
the nested qcow2 by attaching the outter image to a VM and running "qemu-img
create -f qcow2 /dev/vda" in guest shell. Kevin?

Fam

Re: [Qemu-devel] qxl : no mouse cursor with sdl2/gtk UI

[Gerd Hoffmann]

On Do, 2016-01-21 at 10:41 +0100, nicolas prochazka wrote:
> hello, 
> 
> yes you 're right 
> 
> - windows seven, qxl , usb mouse passthrough  : no mouse cursor

Try dropping the usb tablet from the libvirt config then.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] Add optionrom compatible with fw_cfg DMA version

[Stefan Hajnoczi]

On Tue, Jan 19, 2016 at 01:11:47PM +0100, Gerd Hoffmann wrote:
> > If you really think they should be merged, I'd even propose to
> > merge the ASM version onto the C version (convert this patch into
> > linuxboot.S). This slightly improves readability.
> 
> Fully agree.  I'm personally fine with having two roms, but when merging
> them into one we surely should ditch the fw_cfg asm macros and go with
> something more maintainable.

There is no technical requirement for a unified linuxboot ROM.  If there
is no disadvantage to having 2 ROMs then let's stick to Marc's approach.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] pseries: Allow TCG h_enter to work with hotplugged memory

[Paolo Bonzini]

On 21/01/2016 04:50, David Gibson wrote:
> On Thu, Jan 21, 2016 at 12:48:46PM +1100, Alexey Kardashevskiy
> wrote:
>> On 01/21/2016 12:41 PM, David Gibson wrote:
>>> The implementation of the H_ENTER hypercall for PAPR guests
>>> needs to enforce correct access attributes on the inserted
>>> HPTE.  This means determining if the HPTE's real address is a
>>> regular RAM address (which requires attributes for coherent
>>> access) or an IO address (which requires attributes for
>>> cache-inhibited access).
>>> 
>>> At the moment this check is implemented with (raddr <
>>> machine->ram_size), but that only handles addresses in the base
>>> RAM area, not any hotplugged RAM.
>>> 
>>> This patch corrects the problem with a new helper.
>>> 
>>> Signed-off-by: David Gibson 
>> 
>> 
>> Reviewed-by: Alexey Kardashevskiy 
> 
> Thanks, merged to ppc-for-2.6.

Can you still remove the Pascal parentheses? :)

Paolo

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Gerd Hoffmann]

  Hi,

> That sounds good in theory (but see below) since AioContext integrates
> with the glib main loop because it is a GSource.  QEMU code should use
> AioContext where we have high resolution timers and APIs for file
> descriptor, EventNotifier, and BH.

Sounds good.

> Use BH or EventNotifier instead.  BH has optimizations to avoid syscalls
> where possible while EventNotifier is just a plain eventfd/pipe
> (requires a write() syscall for each notification).

How does BH signaling work?  I know I can qemu_bh_schedule from !main
thread to kick BH in main thread context.  The other way around works
using aio_bh_new + aio_bh_call I guess?

thanks,
  Gerd

Re: [Qemu-devel] qxl : no mouse cursor with sdl2/gtk UI

[nicolas prochazka]

it does not work , with dropping usb tablet.
to be more precise, when i write "no mouse cursor', it means, mouse is ok (
click and position) , it's just the mouse pointer which is not visible

Regard,
Nicolas

2016-01-21 10:54 GMT+01:00 Gerd Hoffmann :

> On Do, 2016-01-21 at 10:41 +0100, nicolas prochazka wrote:
> > hello,
> >
> > yes you 're right
> >
> > - windows seven, qxl , usb mouse passthrough  : no mouse cursor
>
> Try dropping the usb tablet from the libvirt config then.
>
> cheers,
>   Gerd
>
>
>

Re: [Qemu-devel] [PATCH] net: set endianness on all backend devices

[Laurent Vivier]

ping

[added Jason in cc:]

On 13/01/2016 20:26, Laurent Vivier wrote:
> commit 5be7d9f1b1452613b95c6ba70b8d7ad3d0797991
>vhost-net: tell tap backend about the vnet endianness
> 
> makes vhost net to set the endianness of the device, but only for
> the first device.
> 
> In case of multiqueue, we have multiple devices... This patch sets the
> endianness for all the devices of the interface.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  hw/net/vhost_net.c | 23 +++
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 318c3e6..10e233a 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -300,21 +300,19 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> *ncs,
>  BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev)));
>  VirtioBusState *vbus = VIRTIO_BUS(qbus);
>  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
> -int r, e, i;
> +int r, e, i, j;
>  
>  if (!k->set_guest_notifiers) {
>  error_report("binding does not support guest notifiers");
> -r = -ENOSYS;
> -goto err;
> +return -ENOSYS;
>  }
>  
> -r = vhost_net_set_vnet_endian(dev, ncs[0].peer, true);
> -if (r < 0) {
> -goto err;
> -}
> -
> -for (i = 0; i < total_queues; i++) {
> -vhost_net_set_vq_index(get_vhost_net(ncs[i].peer), i * 2);
> +for (j = 0; j < total_queues; j++) {
> +r = vhost_net_set_vnet_endian(dev, ncs[j].peer, true);
> +if (r < 0) {
> +goto err_endian;
> +}
> +vhost_net_set_vq_index(get_vhost_net(ncs[j].peer), j * 2);
>  }
>  
>  r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
> @@ -343,8 +341,9 @@ err_start:
>  fflush(stderr);
>  }
>  err_endian:
> -vhost_net_set_vnet_endian(dev, ncs[0].peer, false);
> -err:
> +while (--j >= 0) {
> +vhost_net_set_vnet_endian(dev, ncs[j].peer, false);
> +}
>  return r;
>  }
>  
> 

Re: [Qemu-devel] [PATCH] linux-user: add option to intercept execve() syscalls

[Laurent Vivier]

Le 18/01/2016 05:33, Petros Angelatos a écrit :
> From: Petros Angelatos 
> 
> In order for one to use QEMU user mode emulation under a chroot, it is
> required to use binfmt_misc. This can be avoided by QEMU never doing a
> raw execve() to the host system.
> 
> Introduce a new option, -execve=path, that sets the absolute path to the
> QEMU interpreter and enables execve() interception. When a guest process
> tries to call execve(), qemu_execve() is called instead.
> 
> qemu_execve() will prepend the interpreter set with -execve, similar to
> what binfmt_misc would do, and then pass the modified execve() to the
> host.
> 
> It is necessary to parse hashbang scripts in that function otherwise
> the kernel will try to run the interpreter of a script without QEMU and
> get an invalid exec format error.
> 
> Signed-off-by: Petros Angelatos 
> ---
>  linux-user/main.c|   8 
>  linux-user/qemu.h|   1 +
>  linux-user/syscall.c | 111 
> ++-
>  3 files changed, 119 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index ee12035..5951279 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -79,6 +79,7 @@ static void usage(int exitcode);
>  
>  static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
>  const char *qemu_uname_release;
> +const char *qemu_execve_path;
>  
>  /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
> we allocate a bigger stack. Need a better solution, for example
> @@ -3828,6 +3829,11 @@ static void handle_arg_guest_base(const char *arg)
>  have_guest_base = 1;
>  }
>  
> +static void handle_arg_execve(const char *arg)
> +{
> +qemu_execve_path = strdup(arg);

I think you can use the parameter just as an on/off switch and
realpath(argv[0]) as qemu_execve_path.

I don't see any reason to use other binary than the one in use.

> +}
> +
>  static void handle_arg_reserved_va(const char *arg)
>  {
>  char *p;
> @@ -3913,6 +3919,8 @@ static const struct qemu_argument arg_table[] = {
>   "uname",  "set qemu uname release string to 'uname'"},
>  {"B",  "QEMU_GUEST_BASE",  true,  handle_arg_guest_base,
>   "address","set guest_base address to 'address'"},
> +{"execve", "QEMU_EXECVE",  true,   handle_arg_execve,
> + "path",   "use interpreter at 'path' when a process calls 
> execve()"},
>  {"R",  "QEMU_RESERVED_VA", true,  handle_arg_reserved_va,
>   "size",   "reserve 'size' bytes for guest virtual address space"},
>  {"d",  "QEMU_LOG", true,  handle_arg_log,
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index bd90cc3..0d9b058 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -140,6 +140,7 @@ void init_task_state(TaskState *ts);
>  void task_settid(TaskState *);
>  void stop_all_tasks(void);
>  extern const char *qemu_uname_release;
> +extern const char *qemu_execve_path;
>  extern unsigned long mmap_min_addr;
>  
>  /* ??? See if we can avoid exposing so much of the loader internals.  */
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0cbace4..d0b5442 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5854,6 +5854,109 @@ static target_timer_t get_timer_id(abi_long arg)
>  return timerid;
>  }
>  
> +#define BINPRM_BUF_SIZE 128

This is defined in 

> +/* qemu_execve() Must return target values and target errnos. */
> +static abi_long qemu_execve(char *filename, char *argv[],
> +  char *envp[])
> +{
> +char *i_arg = NULL, *i_name = NULL;
> +char **new_argp;
> +int argc, fd, ret, i, offset = 3;
> +char *cp;
> +char buf[BINPRM_BUF_SIZE];
> +
> +for (argc = 0; argv[argc] != NULL; argc++) {
> +/* nothing */ ;
> +}
> +
> +fd = open(filename, O_RDONLY);
> +if (fd == -1) {
> +return -ENOENT;

return -errno; ?

> +}
> +
> +ret = read(fd, buf, BINPRM_BUF_SIZE);
> +if (ret == -1) {
> +close(fd);
> +return -ENOENT;

return -errno; ?

> +}
> +
> +close(fd);
> +
> +/* adapted from the kernel
> + * 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/binfmt_script.c
> + */
> +if ((buf[0] == '#') && (buf[1] == '!')) {

what happens if read() < 2 ?

> +/*
> + * This section does the #! interpretation.
> + * Sorta complicated, but hopefully it will work.  -TYT
> + */
> +
> +buf[BINPRM_BUF_SIZE - 1] = '\0';
> +cp = strchr(buf, '\n');
> +if (cp == NULL) {
> +cp = buf+BINPRM_BUF_SIZE-1;
> +}
> +*cp = '\0';
> +while (cp > buf) {
> +cp--;
> +if ((*cp == ' ') || (*cp == '\t')) {
> +*cp = '\0';
> +} else {
> +break;
> +}
> +}
> +for (cp = buf+2; (*cp == ' ') || 

Re: [Qemu-devel] [PATCH v7] spec: add qcow2 bitmaps extension specification

[Vladimir Sementsov-Ogievskiy]

On 21.01.2016 12:53, Kevin Wolf wrote:

Am 21.01.2016 um 09:22 hat Vladimir Sementsov-Ogievskiy geschrieben:

On 21.01.2016 00:22, John Snow wrote:

On 01/20/2016 07:34 AM, Vladimir Sementsov-Ogievskiy wrote:

On 19.01.2016 20:48, Kevin Wolf wrote:

Am 11.01.2016 um 14:05 hat Vladimir Sementsov-Ogievskiy geschrieben:

The new feature for qcow2: storing bitmaps.

This patch adds new header extension to qcow2 - Bitmaps Extension. It
provides an ability to store virtual disk related bitmaps in a qcow2
image. For now there is only one type of such bitmaps: Dirty Tracking
Bitmap, which just tracks virtual disk changes from some moment.

Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
should be stored in this qcow2 file. The size of each bitmap
(considering its granularity) is equal to virtual disk size.

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

v7:

- Rewordings, grammar.
Max, Eric, John, thank you very much.

- add last paragraph: remaining bits in bitmap data clusters must be
zero.

- s/Bitmap Directory/bitmap directory/ and other names like this at
the request of Max.

v6:

- reword bitmap_directory_size description
- bitmap type: make 0 reserved
- extra_data_size: resize to 4bytes
Also, I've marked this field as "must be zero". We can always change
it, if we decide allowing managing app to specify any extra data, by
defining some magic value as a top of user extra data.. So, for now
non zeor extra_data_size should be considered as an error.
- swap name and extra_data to give good alignment to extra_data.


v5:

- 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
bitmaps.
- rewordings
- move upper bounds to "Notes about Qemu limits"
- s/should/must somewhere. (but not everywhere)
- move name_size field closer to name itself in bitmap header
- add extra data area to bitmap header
- move bitmap data description to separate section

   docs/specs/qcow2.txt | 172
++-
   1 file changed, 171 insertions(+), 1 deletion(-)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..997239d 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -103,7 +103,18 @@ in the description of a field.
   write to an image with unknown auto-clear
features if it
   clears the respective bits from this field first.
   -Bits 0-63:  Reserved (set to 0)
+Bit 0:  Bitmaps extension bit
+This bit indicates consistency for
the bitmaps
+extension data.
+
+It is an error if this bit is set
without the
+bitmaps extension present.
+
+If the bitmaps extension is present
but this
+bit is unset, the bitmaps extension
data is
+inconsistent.

It may as well be consistent, but we don't know.

Perhaps something like "must be considered inconsistent" or "is
potentially inconsistent".


+
+Bits 1-63:  Reserved (set to 0)
  96 -  99:  refcount_order
   Describes the width of a reference count block
entry (width
@@ -123,6 +134,7 @@ be stored. Each extension has a structure like
the following:
   0x - End of the header extension area
   0xE2792ACA - Backing file format name
   0x6803f857 - Feature name table
+0x23852875 - Bitmaps extension
   other  - Unknown header extension, can
be safely
ignored
   @@ -166,6 +178,34 @@ the header extension data. Each entry look
like this:
   terminated if it has full length)
 +== Bitmaps extension ==
+
+The bitmaps extension is an optional header extension. It provides
the ability
+to store bitmaps related to a virtual disk. For now, there is only
one bitmap
+type: the dirty tracking bitmap, which tracks virtual disk changes

>from some

+point in time.

I have one major problem with this patch, and it starts here.

The spec talks about dirty tracking bitmaps all the way, but it never
really defines what a dirty tracking bitmap even contains. It has a few
hints here and there, but they aren't consistent.

Here's the first hint: They track "virtual disk changes", which implies
they track guest clusters rather than host clusters.


+The data of the extension should be considered consistent only if the
+corresponding auto-clear feature bit is set, see autoclear_features
above.
+
+The fields of the bitmaps extension are:
+
+  0 -  3:  nb_bitmaps
+   The number of bitmaps contained in the image.
Must be
+   greater than or equal to 1.
+
+   

Re: [Qemu-devel] [PATCH v7] spec: add qcow2 bitmaps extension specification

[Vladimir Sementsov-Ogievskiy]

On 21.01.2016 00:22, John Snow wrote:


On 01/20/2016 07:34 AM, Vladimir Sementsov-Ogievskiy wrote:

On 19.01.2016 20:48, Kevin Wolf wrote:

Am 11.01.2016 um 14:05 hat Vladimir Sementsov-Ogievskiy geschrieben:

The new feature for qcow2: storing bitmaps.

This patch adds new header extension to qcow2 - Bitmaps Extension. It
provides an ability to store virtual disk related bitmaps in a qcow2
image. For now there is only one type of such bitmaps: Dirty Tracking
Bitmap, which just tracks virtual disk changes from some moment.

Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
should be stored in this qcow2 file. The size of each bitmap
(considering its granularity) is equal to virtual disk size.

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

v7:

- Rewordings, grammar.
Max, Eric, John, thank you very much.

- add last paragraph: remaining bits in bitmap data clusters must be
zero.

- s/Bitmap Directory/bitmap directory/ and other names like this at
the request of Max.

v6:

- reword bitmap_directory_size description
- bitmap type: make 0 reserved
- extra_data_size: resize to 4bytes
Also, I've marked this field as "must be zero". We can always change
it, if we decide allowing managing app to specify any extra data, by
defining some magic value as a top of user extra data.. So, for now
non zeor extra_data_size should be considered as an error.
- swap name and extra_data to give good alignment to extra_data.


v5:

- 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
bitmaps.
- rewordings
- move upper bounds to "Notes about Qemu limits"
- s/should/must somewhere. (but not everywhere)
- move name_size field closer to name itself in bitmap header
- add extra data area to bitmap header
- move bitmap data description to separate section

   docs/specs/qcow2.txt | 172
++-
   1 file changed, 171 insertions(+), 1 deletion(-)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index 121dfc8..997239d 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -103,7 +103,18 @@ in the description of a field.
   write to an image with unknown auto-clear
features if it
   clears the respective bits from this field first.
   -Bits 0-63:  Reserved (set to 0)
+Bit 0:  Bitmaps extension bit
+This bit indicates consistency for
the bitmaps
+extension data.
+
+It is an error if this bit is set
without the
+bitmaps extension present.
+
+If the bitmaps extension is present
but this
+bit is unset, the bitmaps extension
data is
+inconsistent.

It may as well be consistent, but we don't know.

Perhaps something like "must be considered inconsistent" or "is
potentially inconsistent".


+
+Bits 1-63:  Reserved (set to 0)
  96 -  99:  refcount_order
   Describes the width of a reference count block
entry (width
@@ -123,6 +134,7 @@ be stored. Each extension has a structure like
the following:
   0x - End of the header extension area
   0xE2792ACA - Backing file format name
   0x6803f857 - Feature name table
+0x23852875 - Bitmaps extension
   other  - Unknown header extension, can
be safely
ignored
   @@ -166,6 +178,34 @@ the header extension data. Each entry look
like this:
   terminated if it has full length)
 +== Bitmaps extension ==
+
+The bitmaps extension is an optional header extension. It provides
the ability
+to store bitmaps related to a virtual disk. For now, there is only
one bitmap
+type: the dirty tracking bitmap, which tracks virtual disk changes
from some
+point in time.

I have one major problem with this patch, and it starts here.

The spec talks about dirty tracking bitmaps all the way, but it never
really defines what a dirty tracking bitmap even contains. It has a few
hints here and there, but they aren't consistent.

Here's the first hint: They track "virtual disk changes", which implies
they track guest clusters rather than host clusters.


+The data of the extension should be considered consistent only if the
+corresponding auto-clear feature bit is set, see autoclear_features
above.
+
+The fields of the bitmaps extension are:
+
+  0 -  3:  nb_bitmaps
+   The number of bitmaps contained in the image.
Must be
+   greater than or equal to 1.
+
+   Note: Qemu currently only supports up to 65535
bitmaps per
+   image.
+
+  4 -  7:  

Re: [Qemu-devel] [PATCH v2 02/17] crypto: add cryptographic random byte source

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 02:12:06PM +0800, Fam Zheng wrote:
> On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > +int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
> > + size_t buflen G_GNUC_UNUSED,
> > + Error **errp)
> > +{
> > +error_setg(errp, "No random byte source provided in this build");
> > +return -1;
> 
> Curious, why does a random(3) or /dev/random implementation fall short to
> error?

random(3) does not provide crytographically strong random numbers.

/dev/random would be suitable, but if we read that directly we would
deplete entropy very quickly.

The gnutls backends will use /dev/random in combination with a
cryptographically secure generator to provide high entropy
numbers, at a high data rate.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v3 09/13] nbd: pick first exported volume if no export name is requested

[Paolo Bonzini]

On 19/01/2016 17:44, Daniel P. Berrange wrote:
>> > As a first reaction, I would really avoid magic unless the server
>> > provides a single exports.  But even in that case, I would prefer to
>> > have some synchronization between the server and client command-line.
>> > 
>> > Is an empty NBD_OPT_EXPORT_NAME valid?  What about using new-style
>> > negotiation with empty NBD_OPT_EXPORT_NAME if TLS is requested?
> The main goal here is to ensure the NBD client gets a decent error
> message if it tries to connect without TLS. Even if we are using
> the fixed new style protocol, the client code will send
> NBD_OPT_EXPORT_NAME as the first thing it does. Thanks to a bit of
> crazyness is the NBD protocol spec, the server is unable to reply
> with an error message to NBD_OPT_EXPORT_NAME.
> 
> So if the client connected to a server reqiuring TLS and does not
> request TLS enablement, the server will have no choice but to just
> close the connection with no error. I think this will be pretty
> nasty for users trying to debug problems with TLS.

That's fine.  I'm just not sold on using the first answer from
NBD_OPT_LIST as the argument to the subsequent NBD_OPT_EXPORT_NAME.

In other words, I would prefer to do the following for no export name:

1) server, no TLS: accept either old-style negotiation or new-style
negotation with an empty ("") export name; NBD_OPT_LIST returns a single
export name, "".

2) server, TLS: accept only new-style negotiation with an empty ("")
export name; NBD_OPT_LIST returns a single export name, "".

3) client, no TLS: use old-style negotiation; if the server rejects
old-style negotiation, mention the possibility that the server requires TLS

4) client, TLS: use new-style negotiation with an empty ("") export name.

The only interesting case for named exports is client, no TLS.  Then you
can just send a dummy NBD_OPT_LIST unconditionally, and use the result
to provide a good error message if the server requires TLS.  If it makes
the code simpler to use NBD_OPT_LIST always, even if the client supports
TLS (making the sequence NBD_OPT_STARTTLS, NBD_OPT_LIST,
NBD_OPT_EXPOR_NAME), then that's fine too.

Paolo

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Fam Zheng]

On Mon, 01/18 10:54, Gerd Hoffmann wrote:
>   Hi folks,
> 
> I'm starting to investigate if and how we can move the user interface
> code into its own thread instead of running it in the iothread and
> therefore avoid blocking the guest in case some UI actions take a little
> longer.
> 
> opengl and toolkits tend to be bad at multithreading.  So my idea is to
> have a single thread dedicated to all the UI + rendering stuff, possibly
> let even the virglrenderer run in ui thread context.
> 
> I think we have to make that opt-in per user interface, so we can go
> forward step by step.
> 
> The ui thread will need quite some stuff provided by the mainloop.  Wait
> for all kinds of events (from vnc socket, x11 connection, ...).
> Probably timers too.  Wait for events from other threads (guest screen
> updates).
> 
> Suggestions how to tackle that?
> Can I reuse the qemu mainloop code outside of the iothread?
> Maybe it'll be better to go straight for a glib main loop?
> Is it possible to wait for file handle events and posix condition
> variables at the same time?
> 
> Other notes / hints / suggestions / ideas?

What are taking long time? Can you give a few examples?

Fam

Re: [Qemu-devel] virtio-scsi/blk dataplane and guest memory allocation

[Paolo Bonzini]

On 20/01/2016 21:12, Roy Shterman wrote:
> Hi,
> 
> I have two questions,
> 
> First, I'm developing for Libiscsi and trying to work with virtio-scsi
> dataplane or even virtio-blk dataplane and it doesn't works well.
> 
> I'm working with latest qemu and latest Libiscsi in RedHat 7 libvirt
> package.
> 
> my iscsi xml part is : 
> 
> virtio-blk - 
> 
> 
>   
>   
> 
>   
>   
>function='0x0'/>
> 
> 
> virtio-scsi - 
> 
> 
>   
>   
>   
>   
>   
> 
> 
>function='0x0'/>
> 

There is now support for dataplane in libvirt.  See
https://libvirt.org/formatdomain.html#elementsIOThreadsAllocation and
then you can add an iothread='NN' (NN is a number) to the  element.

> second thing, I'm trying to look for the code where QEMU allocate all
> guest memory (2 GB) in my case.

Start at memory_allocate_system_memory; ultimately you'll reach
qemu_anon_ram_alloc which is basically an mmap.

paolo

Re: [Qemu-devel] [PATCH v2 11/17] qcow2: make qcow2_encrypt_sectors encrypt in place

[Fam Zheng]

On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> Instead of requiring separate input/output buffers for
> encrypting data, change qcow2_encrypt_sectors() to assume
> use of a single buffer, encrypting in place. The current
> callers all used the same buffer for input/output already.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/qcow2-cluster.c | 17 +
>  block/qcow2.c |  5 ++---
>  block/qcow2.h |  3 +--
>  3 files changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 34112c3..f5bc4f2 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -341,12 +341,8 @@ static int count_contiguous_clusters_by_type(int 
> nb_clusters,
>  return i;
>  }
>  
> -/* The crypt function is compatible with the linux cryptoloop
> -   algorithm for < 4 GB images. NOTE: out_buf == in_buf is
> -   supported */
>  int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
> -  uint8_t *out_buf, const uint8_t *in_buf,
> -  int nb_sectors, bool enc,
> +  uint8_t *buf, int nb_sectors, bool enc,
>Error **errp)
>  {
>  union {
> @@ -366,14 +362,12 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
> sector_num,
>  }
>  if (enc) {
>  ret = qcrypto_cipher_encrypt(s->cipher,
> - in_buf,
> - out_buf,
> + buf, buf,
>   512,
>   errp);
>  } else {
>  ret = qcrypto_cipher_decrypt(s->cipher,
> - in_buf,
> - out_buf,
> + buf, buf,
>   512,
>   errp);
>  }
> @@ -381,8 +375,7 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
> sector_num,
>  return -1;
>  }
>  sector_num++;
> -in_buf += 512;
> -out_buf += 512;
> +buf += 512;
>  }
>  return 0;
>  }
> @@ -430,7 +423,7 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
>  Error *err = NULL;
>  assert(s->cipher);
>  if (qcow2_encrypt_sectors(s, start_sect + n_start,
> -  iov.iov_base, iov.iov_base, n,
> +  iov.iov_base, n,
>true, ) < 0) {
>  ret = -EIO;
>  error_free(err);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index d992e7f..2fae692 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1504,7 +1504,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
> *bs, int64_t sector_num,
>  assert(s->cipher);
>  Error *err = NULL;
>  if (qcow2_encrypt_sectors(s, sector_num,  cluster_data,
> -  cluster_data, cur_nr_sectors, 
> false,
> +  cur_nr_sectors, false,
>) < 0) {
>  error_free(err);
>  ret = -EIO;
> @@ -1604,8 +1604,7 @@ static coroutine_fn int 
> qcow2_co_writev(BlockDriverState *bs,
>  qemu_iovec_to_buf(_qiov, 0, cluster_data, hd_qiov.size);
>  
>  if (qcow2_encrypt_sectors(s, sector_num, cluster_data,
> -  cluster_data, cur_nr_sectors,
> -  true, ) < 0) {
> +  cur_nr_sectors, true, ) < 0) {
>  error_free(err);
>  ret = -EIO;
>  goto fail;
> diff --git a/block/qcow2.h b/block/qcow2.h
> index a063a3c..ae04285 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -540,8 +540,7 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int 
> l1_index);
>  void qcow2_l2_cache_reset(BlockDriverState *bs);
>  int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
>  int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
> -  uint8_t *out_buf, const uint8_t *in_buf,
> -  int nb_sectors, bool enc, Error **errp);
> +  uint8_t *buf, int nb_sectors, bool enc, Error 
> **errp);
>  
>  int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
>  int *num, uint64_t *cluster_offset);
> -- 
> 2.5.0
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Stefan Hajnoczi]

On Tue, Jan 19, 2016 at 01:51:09PM +0100, Kevin Wolf wrote:
> Am 18.01.2016 um 10:54 hat Gerd Hoffmann geschrieben:
> >   Hi folks,
> > 
> > I'm starting to investigate if and how we can move the user interface
> > code into its own thread instead of running it in the iothread and
> > therefore avoid blocking the guest in case some UI actions take a little
> > longer.
> > 
> > opengl and toolkits tend to be bad at multithreading.  So my idea is to
> > have a single thread dedicated to all the UI + rendering stuff, possibly
> > let even the virglrenderer run in ui thread context.
> > 
> > I think we have to make that opt-in per user interface, so we can go
> > forward step by step.
> > 
> > The ui thread will need quite some stuff provided by the mainloop.  Wait
> > for all kinds of events (from vnc socket, x11 connection, ...).
> > Probably timers too.  Wait for events from other threads (guest screen
> > updates).
> > 
> > Suggestions how to tackle that?
> > Can I reuse the qemu mainloop code outside of the iothread?
> 
> That should be possible. The block layer runs additional main loops in
> dataplane threads. I think AioContext is the keyword here, so that you
> process only events in your own UI context.
> 
> I'm copying Stefan who knows this stuff a bit better than me.
> 
> > Maybe it'll be better to go straight for a glib main loop?

That sounds good in theory (but see below) since AioContext integrates
with the glib main loop because it is a GSource.  QEMU code should use
AioContext where we have high resolution timers and APIs for file
descriptor, EventNotifier, and BH.

But I think using multiple glib main loops has limitations/problems.
CCing Daniel Berrange because I vaguely remember this topic being
discussed.

> > Is it possible to wait for file handle events and posix condition
> > variables at the same time?

I don't think POSIX condition variables can be monitored from an event
loop.

Use BH or EventNotifier instead.  BH has optimizations to avoid syscalls
where possible while EventNotifier is just a plain eventfd/pipe
(requires a write() syscall for each notification).


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/1] vl: change QEMU state machine for system reset

[Paolo Bonzini]

On 19/01/2016 08:59, Denis V. Lunev wrote:
> @@ -612,8 +617,10 @@ static const RunStateTransition 
> runstate_transitions_def[] = {
>  
>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>  { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
> +{ RUN_STATE_FINISH_MIGRATE, RUN_STATE_PRELAUNCH },
>  
>  { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
> +{ RUN_STATE_RESTORE_VM, RUN_STATE_PRELAUNCH },
>  
>  { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
>  { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
> @@ -627,20 +634,25 @@ static const RunStateTransition 
> runstate_transitions_def[] = {
>  { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>  
>  { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
> +{ RUN_STATE_SAVE_VM, RUN_STATE_PRELAUNCH },

This should not happen; thus I would remove this line.

>  { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
>  { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
> +{ RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH },
>  
>  { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
>  { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
>  { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
>  { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
> +{ RUN_STATE_SUSPENDED, RUN_STATE_PRELAUNCH },
>  
>  { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>  { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
> +{ RUN_STATE_WATCHDOG, RUN_STATE_PRELAUNCH },
>  
>  { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
>  { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
> +{ RUN_STATE_GUEST_PANICKED, RUN_STATE_PRELAUNCH },
>  
>  { RUN_STATE__MAX, RUN_STATE__MAX },
>  };
> @@ -1886,8 +1899,10 @@ static bool main_loop_should_exit(void)
>  cpu_synchronize_all_states();
>  qemu_system_reset(VMRESET_REPORT);
>  resume_all_vcpus();
> -if (runstate_needs_reset()) {
> -runstate_set(RUN_STATE_PAUSED);
> +if (!runstate_check(RUN_STATE_RUNNING) &&
> +!runstate_check(RUN_STATE_INMIGRATE) &&
> +!runstate_check(RUN_STATE_SAVE_VM)) {

Since SAVE_VM should not happen here, I would leave this check out too.
 If it happens, the lack of a SAVE_VM->PRELAUNCH transition will cause
an assertion failure.

Thanks for the patch!

Paolo

> +runstate_set(RUN_STATE_PRELAUNCH);
>  }
>  }
>  if (qemu_wakeup_requested()) {
> 

Re: [Qemu-devel] qxl : no mouse cursor with sdl2/gtk UI

[nicolas prochazka]

hello,
yes you 're right
- windows seven, qxl , usb mouse passthrough  : no mouse cursor
- windows seven, vga, usb mouse passthrough  : cursor
- windows seven, qxl , no passthrough : cursor

- windows 8, qxl , usb mouse passthrough  : cursor
- windows 8 vga, usb mouse passthrough  : cursor
- windows 8, qxl , no passthrough : cursor

Regards,
Nicolas

2016-01-21 9:03 GMT+01:00 Gerd Hoffmann :

> On Mi, 2016-01-20 at 15:20 +0100, nicolas prochazka wrote:
> > hello,
> >
> > my libvirt definition xml file, if i change qxl by vga , mouse pointer
> > is present ,
> >
> > if i change vm seven to windows81 , mouse pointer is present  ( using
> > wddm qxl driver not the same as seven )
> >
> > with windows seven vm, last qxl , mouse click is ok, but there'is no
> > mouse pointer
> >
> > ( sound card, usb are passthrough device , test without passthrough do
> > not change anyting )
>
> What do you use usb passthrough for?  The mouse?
>
> cheers,
>   Gerd
>
>
>

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Gerd Hoffmann]

On Do, 2016-01-21 at 10:05 +0100, Paolo Bonzini wrote:
> 
> On 21/01/2016 09:44, Dave Airlie wrote:
> > I've hacked on this before, but only with SDL and it was pretty dirty,
> > and it gave a fairly decent
> > speed up.
> > 
> > My thoughts are to use dataplane like design to process the queue in a
> > separate thread,
> > and then have some sort of channel between the UI and virtio-gpu
> > thread to handle things like
> > screen resize etc.
> > 
> > http://cgit.freedesktop.org/~airlied/qemu/commit/?h=dave3d=fe22a0955255afef12b947c4a91efa57e7a7c429
> > is my previous horror patch.
> 
> Instead of having a full-blown thread, are there things (such as the
> TGSI->GLSL conversion) that could be simply offloaded to a userspace
> thread pool, either in QEMU or in virglrenderer?

I think virglrenderer would have to do that.  Unfortunaly opengl isn't
very good at multithreading, so I'm not sure a thread pool would work
well.  Compiling shaders could be a special case where threading
actually works because that isn't in the actual rendering workflow.  Not
fully sure though, Dave?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v9 15/37] qom: Swap 'name' next to visitor in ObjectPropertyAccessor

[Markus Armbruster]

Kevin Wolf  writes:

> Am 21.01.2016 um 10:18 hat Markus Armbruster geschrieben:
>> Eric Blake  writes:
>> > On 01/20/2016 11:49 AM, Markus Armbruster wrote:
>> >> Eric Blake  writes:
>> >> 
>> >> However, the actual match is looser!  For instance, it also matches
>> >> 
>> >> void foo(int *pi, unsigned *pu, void *vp, const char *cp, double 
>> >> **dpp)
>> >> {
>> >> }
>> >
>> > Uggh. My intent was to match exactly 'Object *' and 'Visitor *' as the
>> > first two types, where 'int *' and 'unsigned *' are NOT matches.  But I
>> > don't know Coccinelle well enough to make that blatantly obvious (is my
>> > declaration of 'type Object' not correct?).
>> 
>> 'type Object' makes 'Object' a metavariable matching any C type.
>
> I can't say anything on this one, because I've never used 'type'. You
> may or may not be right. However...
>
>> >> This could mess up unrelated function.  I could double-check it doesn't,
>> >> but I'd rather have a narrower match instead.  Can't give one offhand,
>> >> though.  Ideas?
>> >
>> > Is 'typedef' better than 'type' for constraining the type of the first
>> > two arguments?
>> 
>> Matches any C typedef name.  Less wrong, but still wrong :)
>
> ...I'm pretty sure that this is wrong and 'typedef' only declares that a
> specific type exists as a typedef.

Coccinelle's documentation is awfully terse.  Looks like I jumped to
conclusions.

>> > Or does Coccinelle do literal matches on anything you
>> > don't pre-declare, as in:
>> 
>> Yes, but...
>> 
>> >  @ rule1 @
>> >  identifier fn;
>> >  identifier obj, v, opaque, name, errp;
>> >  @@
>> >   void fn
>> >  - (Object *obj, Visitor *v, void *opaque, const char *name,
>> >  + (Object *obj, Visitor *v, const char *name, void *opaque,
>> > Error **errp) { ... }
>> 
>> ... when I try that, spatch throws a parse error.
>
> Because you need to declare the typedef. :-)
>
> I had a similar problem and asked Julia about it at KVM Forum, so I'm
> pretty sure that it's right.

If I replace 'type' by 'typedef', spatch no longer messes up

void foo(int *pi, unsigned *pu, void *vp, const char *cp, double **dpp)

but still rewrites

void bar(Object *o, Visitor *v, void *opq, const char *n, Error **ep)

Thanks, Kevin!

Eric, could you try to regenerate your patch with the corrected script?

Re: [Qemu-devel] [PATCH v9 19/37] qmp: Fix reference-counting of qnull on empty output visit

[Markus Armbruster]

Eric Blake  writes:

> Commit 6c2f9a15 ensured that we would not return NULL when the
> caller used an output visitor but had nothing to visit. But
> in doing so, it added a FIXME about a reference count leak
> that could abort qemu in the (unlikely) case of SIZE_MAX such
> visits (more plausible on 32-bit).  (Although that commit
> suggested we might fix it in time for 2.5, we ran out of time;
> fortunately, it is unlikely enough to bite that it was not
> worth worrying about during the 2.5 release.)
>
> This fixes things by documenting the internal contracts, and
> explaining why the internal function can return NULL and only
> the public facing interface needs to worry about qnull(),
> thus avoiding over-referencing the qnull_ global object.
>
> It does not, however, fix the stupidity of the stack mixing
> up two separate pieces of information; add a FIXME to explain
> that issue, which will be fixed shortly in a future patch.
>
> Signed-off-by: Eric Blake 
> Cc: qemu-sta...@nongnu.org
> Reviewed-by: Marc-André Lureau 
>
> ---
> v9: enhance commit message
> v8: rebase to earlier changes
> v7: cc qemu-stable, tweak some asserts, drop stale comment, add more
> comments
> v6: no change
> ---
>  qapi/qmp-output-visitor.c   | 39 ---
>  tests/test-qmp-output-visitor.c |  2 ++
>  2 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
> index d367148..316f4e4 100644
> --- a/qapi/qmp-output-visitor.c
> +++ b/qapi/qmp-output-visitor.c
> @@ -29,6 +29,15 @@ typedef QTAILQ_HEAD(QStack, QStackEntry) QStack;
>  struct QmpOutputVisitor
>  {
>  Visitor visitor;
> +/* FIXME: we are abusing stack to hold two separate pieces of
> + * information: the current root object in slot 0, and the stack
> + * of N objects still being built in slots 1 through N (for N+1
> + * slots in use).  Worse, our behavior is inconsistent:
> + * qmp_output_add_obj() visiting two top-level scalars in a row
> + * discards the first in favor of the second, but visiting two
> + * top-level objects in a row tries to append the second object
> + * into the first (since the first object was placed in the stack
> + * in both slot 0 and 1, but only popped from slot 1).  */
>  QStack stack;
>  };
>
> @@ -41,10 +50,12 @@ static QmpOutputVisitor *to_qov(Visitor *v)
>  return container_of(v, QmpOutputVisitor, visitor);
>  }
>
> +/* Push @value onto the stack of current QObjects being built */
>  static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
>  {
>  QStackEntry *e = g_malloc0(sizeof(*e));
>
> +assert(value);
>  e->value = value;
>  if (qobject_type(e->value) == QTYPE_QLIST) {
>  e->is_list_head = true;
> @@ -52,44 +63,51 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, 
> QObject *value)
>  QTAILQ_INSERT_HEAD(>stack, e, node);
>  }
>
> +/* Grab and remove the most recent QObject from the stack */
>  static QObject *qmp_output_pop(QmpOutputVisitor *qov)
>  {
>  QStackEntry *e = QTAILQ_FIRST(>stack);
>  QObject *value;
> +
> +assert(e);
>  QTAILQ_REMOVE(>stack, e, node);
>  value = e->value;
>  g_free(e);
>  return value;

@value cannot be null here, because we never push nulls.  Worth an
assertion, just to help readers?

>  }
>
> +/* Grab the root QObject, if any, in preparation to empty the stack */

Why is this "in preparation to empty the stack"?

>  static QObject *qmp_output_first(QmpOutputVisitor *qov)
>  {
>  QStackEntry *e = QTAILQ_LAST(>stack, QStack);
>
> -/*
> - * FIXME Wrong, because qmp_output_get_qobject() will increment
> - * the refcnt *again*.  We need to think through how visitors
> - * handle null.
> - */
>  if (!e) {
> -return qnull();
> +/* No root */
> +return NULL;
>  }
> -
> +assert(e->value);
>  return e->value;
>  }

Two callers to check: qmp_output_get_qobject() and
qmp_output_visitor_cleanup().  See below.

>
> +/* Grab the most recent QObject from the stack, which must exist */
>  static QObject *qmp_output_last(QmpOutputVisitor *qov)
>  {
>  QStackEntry *e = QTAILQ_FIRST(>stack);
> +
> +assert(e && e->value);
>  return e->value;
>  }
>
> +/* Add @value to the current QObject being built.
> + * If the stack is visiting a dictionary or list, @value is now owned
> + * by that container. Otherwise, @value is now the root.  */
>  static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
> QObject *value)
>  {
>  QObject *cur;
>
>  if (QTAILQ_EMPTY(>stack)) {
> +/* Stack was empty, track this object as root */
>  qmp_output_push_obj(qov, value);
>  return;
>  }
> @@ -98,13 +116,17 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov, 
> const char *name,
>
>  switch 

Re: [Qemu-devel] RFC: running the user interface in a thread ...

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 09:58:11AM +, Stefan Hajnoczi wrote:
> On Tue, Jan 19, 2016 at 01:51:09PM +0100, Kevin Wolf wrote:
> > Am 18.01.2016 um 10:54 hat Gerd Hoffmann geschrieben:
> > >   Hi folks,
> > > 
> > > I'm starting to investigate if and how we can move the user interface
> > > code into its own thread instead of running it in the iothread and
> > > therefore avoid blocking the guest in case some UI actions take a little
> > > longer.
> > > 
> > > opengl and toolkits tend to be bad at multithreading.  So my idea is to
> > > have a single thread dedicated to all the UI + rendering stuff, possibly
> > > let even the virglrenderer run in ui thread context.
> > > 
> > > I think we have to make that opt-in per user interface, so we can go
> > > forward step by step.
> > > 
> > > The ui thread will need quite some stuff provided by the mainloop.  Wait
> > > for all kinds of events (from vnc socket, x11 connection, ...).
> > > Probably timers too.  Wait for events from other threads (guest screen
> > > updates).
> > > 
> > > Suggestions how to tackle that?
> > > Can I reuse the qemu mainloop code outside of the iothread?
> > 
> > That should be possible. The block layer runs additional main loops in
> > dataplane threads. I think AioContext is the keyword here, so that you
> > process only events in your own UI context.
> > 
> > I'm copying Stefan who knows this stuff a bit better than me.
> > 
> > > Maybe it'll be better to go straight for a glib main loop?
> 
> That sounds good in theory (but see below) since AioContext integrates
> with the glib main loop because it is a GSource.  QEMU code should use
> AioContext where we have high resolution timers and APIs for file
> descriptor, EventNotifier, and BH.
> 
> But I think using multiple glib main loops has limitations/problems.
> CCing Daniel Berrange because I vaguely remember this topic being
> discussed.

WRT to GTK you need to be very careful to ensure all gtk_* and
gdk_* API calls are made from a single thread.  Running multiple
glib main loops isn't per-se a problem, as long as you respect
that threading requirement. FWIW, spice already has its own
background thread that runs its own main loop.

> > > Is it possible to wait for file handle events and posix condition
> > > variables at the same time?
> 
> I don't think POSIX condition variables can be monitored from an event
> loop.

Yep, you can't monitor condition vars from poll()

> Use BH or EventNotifier instead.  BH has optimizations to avoid syscalls
> where possible while EventNotifier is just a plain eventfd/pipe
> (requires a write() syscall for each notification).


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|