Re: [Qemu-devel] [PATCH v5 2/5] acpi: move generic aml building helpers into dedictated file

2015-01-27 Thread Michael S. Tsirkin
On Wed, Jan 21, 2015 at 09:09:10AM +, Igor Mammedov wrote:
 the will be later used for composing AML primitives
 and all that could be reused later for ARM machines
 as well.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com

OK so the only thing holding this up is really that
you have indicated that in the follow-up patches
you want to add here more stuff with
aml_ prefix instead of acpi_.

In that case let's name this file aml-build.c?


 ---
 v3:
   * rename acpi_gen_utils.[ch] to acpi-build-utils.[ch]
   * copy GLP license block from acpi-build.c
 v2:
   * fix wrong ident in moved code
 ---
  hw/acpi/Makefile.objs  |   1 +
  hw/acpi/acpi-build-utils.c | 187 
 +
  hw/i386/acpi-build.c   | 162 +---
  include/hw/acpi/acpi-build-utils.h |  23 +
  4 files changed, 213 insertions(+), 160 deletions(-)
  create mode 100644 hw/acpi/acpi-build-utils.c
  create mode 100644 include/hw/acpi/acpi-build-utils.h
 
 diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
 index ee82073..cad0355 100644
 --- a/hw/acpi/Makefile.objs
 +++ b/hw/acpi/Makefile.objs
 @@ -2,3 +2,4 @@ common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o 
 cpu_hotplug.o
  common-obj-$(CONFIG_ACPI) += memory_hotplug.o
  common-obj-$(CONFIG_ACPI) += acpi_interface.o
  common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
 +common-obj-$(CONFIG_ACPI) += acpi-build-utils.o
 diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c
 new file mode 100644
 index 000..79aa610
 --- /dev/null
 +++ b/hw/acpi/acpi-build-utils.c
 @@ -0,0 +1,187 @@
 +/* Support for generating ACPI tables and passing them to Guests
 + *
 + * Copyright (C) 2014 Red Hat Inc
 + *
 + * Author: Michael S. Tsirkin m...@redhat.com
 + * Author: Igor Mammedov imamm...@redhat.com
 + *
 + * This program 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 program 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 General Public License for more details.
 +
 + * You should have received a copy of the GNU General Public License along
 + * with this program; if not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include stdio.h
 +#include stdarg.h
 +#include assert.h
 +#include stdbool.h
 +#include hw/acpi/acpi-build-utils.h
 +
 +GArray *build_alloc_array(void)
 +{
 +return g_array_new(false, true /* clear */, 1);
 +}
 +
 +void build_free_array(GArray *array)
 +{
 +g_array_free(array, true);
 +}
 +
 +void build_prepend_byte(GArray *array, uint8_t val)
 +{
 +g_array_prepend_val(array, val);
 +}
 +
 +void build_append_byte(GArray *array, uint8_t val)
 +{
 +g_array_append_val(array, val);
 +}
 +
 +void build_append_array(GArray *array, GArray *val)
 +{
 +g_array_append_vals(array, val-data, val-len);
 +}
 +
 +#define ACPI_NAMESEG_LEN 4
 +
 +void GCC_FMT_ATTR(2, 3)
 +build_append_nameseg(GArray *array, const char *format, ...)
 +{
 +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 
 */
 +char s[] = ;
 +int len;
 +va_list args;
 +
 +va_start(args, format);
 +len = vsnprintf(s, sizeof s, format, args);
 +va_end(args);
 +
 +assert(len = ACPI_NAMESEG_LEN);
 +
 +g_array_append_vals(array, s, len);
 +/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
 +g_array_append_vals(array, , ACPI_NAMESEG_LEN - len);
 +}
 +
 +/* 5.4 Definition Block Encoding */
 +enum {
 +PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
 +PACKAGE_LENGTH_2BYTE_SHIFT = 4,
 +PACKAGE_LENGTH_3BYTE_SHIFT = 12,
 +PACKAGE_LENGTH_4BYTE_SHIFT = 20,
 +};
 +
 +void build_prepend_package_length(GArray *package, unsigned min_bytes)
 +{
 +uint8_t byte;
 +unsigned length = package-len;
 +unsigned length_bytes;
 +
 +if (length + 1  (1  PACKAGE_LENGTH_1BYTE_SHIFT)) {
 +length_bytes = 1;
 +} else if (length + 2  (1  PACKAGE_LENGTH_3BYTE_SHIFT)) {
 +length_bytes = 2;
 +} else if (length + 3  (1  PACKAGE_LENGTH_4BYTE_SHIFT)) {
 +length_bytes = 3;
 +} else {
 +length_bytes = 4;
 +}
 +
 +/* Force length to at least min_bytes.
 + * This wastes memory but that's how bios did it.
 + */
 +length_bytes = MAX(length_bytes, min_bytes);
 +
 +/* PkgLength is the length of the inclusive length of the data. */
 +length += length_bytes;
 +
 +switch (length_bytes) {
 +case 1:
 +byte = length;
 +build_prepend_byte(package, byte);
 +return;
 +case 4:
 +byte = length  PACKAGE_LENGTH_4BYTE_SHIFT;
 +build_prepend_byte(package, byte);
 + 

Re: [Qemu-devel] [PATCH] block: Simplify a few g_try_malloc() error checks

2015-01-27 Thread Paolo Bonzini


On 27/01/2015 13:25, Markus Armbruster wrote:
 Unlike malloc(), g_try_malloc()  friends return a null pointer only
 on failure, never for a zero size.  Simplify tests for failure
 accordingly.  This helps Coverity see returned null pointers can't be
 dereferenced.  Also makes the code easier to read.

Unfortunately that's not what I see from the source:

gpointer
g_try_malloc (gsize n_bytes)
{
  gpointer mem;

  if (G_LIKELY (n_bytes))
mem = glib_mem_vtable.try_malloc (n_bytes);
  else
mem = NULL;

  TRACE (GLIB_MEM_ALLOC((void*) mem, (unsigned int) n_bytes, 0, 1));

  return mem;
}

Paolo



Re: [Qemu-devel] [PATCH v2 21/47] acpi: add acpi_resource_template() helper

2015-01-27 Thread Claudio Fontana
Hello Igor,

On 22.01.2015 15:50, Igor Mammedov wrote:
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/acpi/acpi-build-utils.c | 8 
  include/hw/acpi/acpi-build-utils.h | 1 +
  2 files changed, 9 insertions(+)
 
 diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c
 index 2d5e77a..32a4377 100644
 --- a/hw/acpi/acpi-build-utils.c
 +++ b/hw/acpi/acpi-build-utils.c
 @@ -493,6 +493,14 @@ AcpiAml GCC_FMT_ATTR(1, 2) acpi_device(const char 
 *name_format, ...)
  return var;
  }
 

 +/* ResourceTemplate marcos helper */

Since you have been so careful about putting references to the spec everywhere 
else,
what about adding something for ResourceTemplate macros too (note typo above)?

For example 19.2.3 ASL Resource Templates if that's the right one. (I am 
looking at version 5.1)

Thanks,

Claudio

 +AcpiAml acpi_resource_template(void)
 +{
 +/* ResourceTemplate is a buffer of Resources with EndTag at the end */
 +AcpiAml var = aml_allocate_internal(0x11 /* BufferOp */, RES_TEMPLATE);
 +return var;
 +}
 +
  /* ACPI 5.0: 20.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
  AcpiAml acpi_buffer(void)
  {
 diff --git a/include/hw/acpi/acpi-build-utils.h 
 b/include/hw/acpi/acpi-build-utils.h
 index a79c085..594fae7 100644
 --- a/include/hw/acpi/acpi-build-utils.h
 +++ b/include/hw/acpi/acpi-build-utils.h
 @@ -46,6 +46,7 @@ AcpiAml acpi_method(const char *name, int arg_count);
  AcpiAml GCC_FMT_ATTR(1, 2) acpi_scope(const char *name_format, ...);
  AcpiAml GCC_FMT_ATTR(1, 2) acpi_device(const char *name_format, ...);
  AcpiAml acpi_buffer(void);
 +AcpiAml acpi_resource_template(void);
  AcpiAml acpi_package(uint8_t num_elements);
  
  /* other helpers */
 






[Qemu-devel] [PULL 05/16] acpi-test: update expected DSDT

2015-01-27 Thread Michael S. Tsirkin
Previous patch
pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled
changed DSDT, update expected test files.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 tests/acpi-test-data/pc/DSDT  | Bin 3592 - 3621 bytes
 tests/acpi-test-data/q35/DSDT | Bin 8182 - 8211 bytes
 tests/acpi-test-data/q35/SSDT | Bin 560 - 560 bytes
 3 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/tests/acpi-test-data/pc/DSDT b/tests/acpi-test-data/pc/DSDT
index 
ee9cc6781cea3a9515b9a176eea3459f8e4d8655..010d74603f09430d65fae1ce2a0d867c1964a1a8
 100644
GIT binary patch
delta 76
zcmeBSt`Tj66_M9%E!RKxMw35D_k`}2c*=Q?gI0YR?89Pu8WEBDGxY#)2Lp@!1
godZG@GV)6ba!S(`ic*X7ON)|Iiy1bvb1||20PCp~jQ{`u

delta 47
zcmZ1~(;s66_Mf!NVBn6iJm6ORyaIzrhbAHW$AlG1ycn?n(9)t@Y@1cM7+C=S
C=n9Ph

diff --git a/tests/acpi-test-data/q35/DSDT b/tests/acpi-test-data/q35/DSDT
index 
ef0c75f4236de3e6f727e21991533f2bf8279dee..8ac32efc0316daeed23e9002aa8bb05fb0fe483f
 100644
GIT binary patch
delta 76
zcmexnKiPrHCDiISb3oFoW7nlzKY}1X=Q?gI0YR?89Pu8WEBDGxY#)2Lp@!1
godZG@GV)6ba!S(`ic*X7ON)|Iiy1a+$ed#a02(wFDgXcg

delta 47
zcmbR2@XemfCDk8n+pFAceYSK(jf|HG;pYv-51i1!t#Cv$U@Hk%JV%zL0bB-AR
DVv7zc

diff --git a/tests/acpi-test-data/q35/SSDT b/tests/acpi-test-data/q35/SSDT
index 
4e45510181c4038505ae1815b76799a2cd0e8808..2cb3e08bd4dcd2998ce5f87641ade4b6ce64c695
 100644
GIT binary patch
delta 26
icmdnMvVnywIM^k`fQf;D@$5vd3Cv7fOdID;W{9RD+bR1

delta 26
icmdnMvVnywIM^k`fQf;D@!~|T3Cs*!3)W8W{9RLk7P

-- 
MST




[Qemu-devel] [PATCH] block: fix off-by-one error in qcow and qcow2

2015-01-27 Thread Jeff Cody
This fixes an off-by-one error introduced in 9a29e18.  Both qcow and
qcow2 need to make sure to leave room for string terminator '\0' for
the backing file, so the max length of the non-terminated string is
either 1023 or PATH_MAX - 1.

Reported-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Jeff Cody jc...@redhat.com
---
 block/qcow.c  | 2 +-
 block/qcow2.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index ccbe9e0..0558969 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -215,7 +215,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 /* read the backing file name */
 if (header.backing_file_offset != 0) {
 len = header.backing_file_size;
-if (len  1023 || len  sizeof(bs-backing_file)) {
+if (len  1023 || len = sizeof(bs-backing_file)) {
 error_setg(errp, Backing file name too long);
 ret = -EINVAL;
 goto fail;
diff --git a/block/qcow2.c b/block/qcow2.c
index dbaf016..7e614d7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -869,7 +869,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, 
int flags,
 if (header.backing_file_offset != 0) {
 len = header.backing_file_size;
 if (len  MIN(1023, s-cluster_size - header.backing_file_offset) ||
-len  sizeof(bs-backing_file)) {
+len = sizeof(bs-backing_file)) {
 error_setg(errp, Backing file name too long);
 ret = -EINVAL;
 goto fail;
-- 
1.9.3




Re: [Qemu-devel] [PATCH v2 0/8] qemu: guest agent: implement guest-exec command for Linux

2015-01-27 Thread Denis V. Lunev

On 31/12/14 16:06, Denis V. Lunev wrote:

hese patches for guest-agent add the functionality to execute commands on
a guest UNIX machine.

These patches add the following interfaces:

guest-pipe-open
guest-exec
guest-exec-status

With these interfaces it's possible to:

* Open an anonymous pipe and work with it as with a file using already
implemented interfaces guest-file-{read,write,flush,close}.

* Execute a binary or a script on a guest machine.
We can pass arbitrary arguments and environment to a new child process.

* Pass redirected IO from/to stdin, stdout, stderr from a child process to a
local file or an anonymous pipe.

* Check the status of a child process, get its exit status if it exited or get
signal number if it was killed.

We plan to add support for Windows in the near future using the same interfaces
we introduce here.

Example of usage:

{execute: guest-pipe-open, arguments:{mode: r}}
{'return': 1000}

{execute:guest-exec, arguments:{ path: id, params: [user],
 env: [MYENV=myvalue], handle_stdout: 1000 }}'
{return: 2636}

{execute: guest-exec-status, arguments: {pid: 2636}}
{return:{exit:0,handle_stderr:-1,handle_stdin:-1,handle_stdout:1000,signal:-1}}

{execute: guest-file-read, arguments: {handle: 1000, count:128}}
{return:{count:58,buf-b64:dWlk...,eof:true}}

{execute: guest-file-close, arguments: {handle: 1000}}
{return:{}}


These patches are based on the patches proposed by Michael Roth in 2011:
http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg00722.html

We made several modifications to the interfaces proposed by Michael to simplify
their usage and we also added several new functions:

* Arguments to an executable file are passed as an array of strings.

 Before that we had to pass parameters like this:
 'params': [ {'param': '-b'}, {'param': '-n1'} ]

 Now we may pass them just as an array of strings:
 'params': [ '-b', '-n1' ]

* Environment can be passed to a child process.

 env: [MYENV=myvalue]

* Removed detach argument from guest-exec - it never waits for a child
process to signal.  With this change it's possible to return just PID from
guest-exec, a user must call guest-exec-status to get the status of a child.

* Removed wait argument from guest-exec-status - waiting for a child process
to signal is dangerous, because it may block the whole qemu-ga.  Instead, the
command polls just once a status of a child.

* guest-exec-status returns exit status of a child process or a signal number,
in case a process was killed.

* Simplified the command guest-pipe-open - the way how it stores the pipe
fd's.  No additional logic is needed about which end of pipe should be closed
with guest-file-close.  This way we avoid modifying the interface of the
existing guest-file-close.

In the conversation about the original patches there was a suggestion to merge
path and params into one single list.  But we didn't do so, because having
separate path is similar to exec*() family functions in UNIX.  That way it
looks more consistent.

Changes from v1:
- Windows version of the patchset is added
- SIGPIPE processing is added for Unix version of the patchset
- added guest_exec_file_busy() to prevent GuestFileHandle* object from being
   deleted in case it's used in guest-exec command.

Signed-off-by: Semen Zolin szo...@parallels.com
Signed-off-by: Olga Krishtal okrish...@parallels.com
Signed-off-by: Denis V. Lunev d...@openvz.org


ping



[Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files

2015-01-27 Thread Denis V. Lunev
fallocate() works fine and could handle properly with arbitrary size
requests. There is no sense to reduce the amount of space to fallocate.
The bigger is the size, the better is the performance as the amount of
journal updates is reduced.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
 block/raw-posix.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index fa05239..e0b35c9 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -292,6 +292,20 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 }
 }
 
+static void raw_probe_max_write_zeroes(BlockDriverState *bs)
+{
+BDRVRawState *s = bs-opaque;
+struct stat st;
+
+if (fstat(s-fd, st)  0) {
+return; /* no problem, keep default value */
+}
+if (!S_ISREG(st.st_mode) || !s-discard_zeroes) {
+return;
+}
+bs-bl.max_write_zeroes = INT_MAX;
+}
+
 static void raw_parse_flags(int bdrv_flags, int *open_flags)
 {
 assert(open_flags != NULL);
@@ -598,6 +612,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 /* Fail already reopen_prepare() if we can't get a working O_DIRECT
  * alignment with the new fd. */
 if (raw_s-fd != -1) {
+raw_probe_max_write_zeroes(state-bs);
 raw_probe_alignment(state-bs, raw_s-fd, local_err);
 if (local_err) {
 qemu_close(raw_s-fd);
@@ -651,6 +666,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 
 raw_probe_alignment(bs, s-fd, errp);
 bs-bl.opt_mem_alignment = s-buf_align;
+
+raw_probe_max_write_zeroes(bs);
 }
 
 static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
-- 
1.9.1




Re: [Qemu-devel] [PATCH v2 1/4] pci: Split pcie_host_mmcfg_map()

2015-01-27 Thread Peter Maydell
On 21 January 2015 at 16:18, Alexander Graf ag...@suse.de wrote:
 The mmcfg space is a memory region that allows access to PCI config space
 in the PCIe world. To maintain abstraction layers, I would like to expose
 the mmcfg space as a sysbus mmio region rather than have it mapped straight
 into the system's memory address space though.

 So this patch splits the initialization of the mmcfg space from the actual
 mapping, allowing us to only have an mmfg memory region without the map.

 Signed-off-by: Alexander Graf ag...@suse.de
 Reviewed-by: Claudio Fontana claudio.font...@huawei.com
 Tested-by: Claudio Fontana claudio.font...@huawei.com

Reviewed-by: Peter Maydell peter.mayd...@linaro.org
...as far as it goes, but:

Really the pcie_host_mmcfg_map/unmap/update() function is just totally
misguided. This functionality should be pushed upwards into
hw/pci-host/q35.c which can handle its own mapping of the MMIO region
into the system address space at the appropriate location/size.

In particular, at the moment q35.c will leak a bunch of stuff
every time the guest unmaps and remaps the mmcfg space, because
we call memory_region_init_io() over and over again on the same
MMIO object (which isn't valid).

Any time you see a device with its own base address in its
device struct it's a red flag that the design's probably wrong...

The size of the MMCFG region should probably be a device property.
Then the subclass realize could just rely on the baseclass realize
to always create the mmio region, rather than having to explicitly
call a function to get it to do the right thing.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v5 1/3] qcow2: Add qcow2_shrink_l1_and_l2_table for qcow2 shrinking

2015-01-27 Thread Jun Li
On Thu, 01/22 14:14, Max Reitz wrote:
 On 2015-01-19 at 08:16, Jun Li wrote:
 On Thu, 01/15 13:47, Max Reitz wrote:
 On 2015-01-03 at 07:23, Jun Li wrote:
 On Fri, 11/21 11:56, Max Reitz wrote:
 So, as for what I think we do need to do when shrinking (and keep in mind:
 The offset given to qcow2_truncate() is the guest size! NOT the host image
 size!):
 
 (1) Determine the first L2 table and the first entry in the table which 
 will
 lie beyond the new guest disk size.
 Here is not correct always. Due to the COW, using offset to calculate the
 first entry of the first L2 table will be incorrect.
 Again: This is *not* about the host disk size or the host offset of some
 cluster, but about the *guest* disk size.
 
 Let's make up an example. You have a 2 GB disk but you want to resize it to
 1.25 GB. The cluster size is 64 kB, therefore we have 2 GB / 64 kB = 32,768
 data clusters (as long as there aren't any internal snapshots, which is a
 prerequisite for resizing qcow2 images).
 
 Every L2 table contains 65,536 / 8 = 8,192 entries; there are thus 32,768 /
 8,192 = 4 L2 tables.
 
 As you can see, one can directly derive the number of data clusters and L2
 tables from the guest disk size (as long as there aren't any internal
 snapshots).
 
 So of course we can do the same for the target disk size: 1.25 GB / 64 kB =
 20,480 data clusters; 20,480 / 8,192 = 2.5 L2 tables, therefore we need
 three L2 tables but only half of the last one (4,096 entries).
 
 Sorry, last time is my mis-understanding. If do not use qcow2_truncate(), I
 think don't existing above issue.
 
 For my original thought, I want to say:
 Sometimes the second L2 table will contain some entry, the pointer in this
 entry will point to a cluster which address is larger than 1.25 GB.
 
 Correct.
 
 So if not use qcow2_truncate(), won't discard above cluster which address is
 larger than 1.25 GB.

Sorry, I do not express my meaning clearly. 

Here I want to say:

As some entry(let call it entry1) will point to a cluster(let call it
cluster1) which address is larger than 1.25GB, so if we use qcow2_truncate()
and will discard this cluster1. So entry1 will have an error after cluster1
discard. If do not use qcow2_truncate(), so won't discard cluster1.

 
 I'm sorry, I can't really follow what you are trying to say here, so I'll
 just try to reply with things that may or may not be what you wanted to talk
 about.
 
 If you are using qemu-img resize and thus subsequently qcow2_truncate() to
 shrink an image, you cannot expect the image to shrink to the specified file
 length, for several reasons.
 
 First, if you shrink it to 1 GB, but only half of that is actually used, the
 image might of course very well have a length below 1 GB.
 
 Second, there is metadata overhead. So if you are changing the guest disk
 size to 1 GB (all of which is occupied), the host file size will exceed 1 GB
 because of that overhead.
 
 Third, I keep repeating myself here, but file length is not file size. So
 you may observe a file length of 10 GB or more because the clusters are
 spread all over the image file. This is something we'd have to combat with
 defragmentation; but the question is whether we really need to (see below
 for more on that). The point is that it doesn't matter whether the image has
 a file length of 10 GB; the file size will be around 1 GB anyway.
 
 But I still have another worry.
 
 Suppose virtual size and disk size are all 2G. After we resize it to
 1.25G, seems we will get virtual size is 1.25G but disk size is still 2G
 
 No, it won't. I can prove it to you:

Yes, you are right. I have double checked my PATCH v5. Seems I don't use
qcow2_process_discards(and this function will call bdrv_discard() to discard
cluster on host) in my patch v5. I will submit a new version of patch. Thanks.

Regards,
Jun Li

 
 $ qemu-img create -f qcow2 test.qcow2 64M
 $ qemu-io -c 'write 0 64M' test.qcow2
 $ qemu-img info test.qcow2
 ...
 disk size: 64M
 ...
 
 Okay, so far it's just what we'd expect. Now let's implement my proposal for
 truncation: Let's assume the image should be shrinked to 32 MB, so we
 discard all clusters starting at 32 MB (guest offset) (which is 64 MB - 32
 MB = 32 MB of data):
 
 $ qemu-io -c 'discard 32M 32M' test.qcow2
 $ qemu-img info test.qcow2
 ...
 disk size: 32M
 ...
 
 Great!
 
 if do not use qcow2_truncate() to truncate the file(Yes, I know use
 qcow2_truncate is not a resolution). This seems strange, not so perfect.
 
 We know that every cluster references somewhere after that limit (that is,
 every entry in the fourth L2 table and every entry starting with index 4,096
 in the third L2 table) is a data cluster with a guest offset somewhere
 beyond 1.25 GB, so we don't need it anymore.
 
 Thus, we simply discard all those data clusters and after that we can
 discard the fourth L2 table. That's it.
 
 If we really want to we can calculate the highest cluster host offset in use
 and truncate the image accordingly. But that's 

Re: [Qemu-devel] [PATCH] block: Simplify a few g_try_malloc() error checks

2015-01-27 Thread Kevin Wolf
Am 27.01.2015 um 14:42 hat Markus Armbruster geschrieben:
 Paolo Bonzini pbonz...@redhat.com writes:
 
  On 27/01/2015 13:25, Markus Armbruster wrote:
  Unlike malloc(), g_try_malloc()  friends return a null pointer only
  on failure, never for a zero size.  Simplify tests for failure
  accordingly.  This helps Coverity see returned null pointers can't be
  dereferenced.  Also makes the code easier to read.
 
  Unfortunately that's not what I see from the source:
 
  gpointer
  g_try_malloc (gsize n_bytes)
  {
gpointer mem;
 
if (G_LIKELY (n_bytes))
  mem = glib_mem_vtable.try_malloc (n_bytes);
else
  mem = NULL;
 
TRACE (GLIB_MEM_ALLOC((void*) mem, (unsigned int) n_bytes, 0, 1));
 
return mem;
  }
 
 You're right.  Brain fart, please ignore.

Should we consider introducing a qemu_try_malloc() that has the desired
behaviour? The (g_try_)malloc() interface is really easy to misuse and
I've messed up some error checks in the block layer myself.

Kevin



Re: [Qemu-devel] [PATCH] block: fix off-by-one error in qcow and qcow2

2015-01-27 Thread Kevin Wolf
Am 27.01.2015 um 14:33 hat Jeff Cody geschrieben:
 This fixes an off-by-one error introduced in 9a29e18.  Both qcow and
 qcow2 need to make sure to leave room for string terminator '\0' for
 the backing file, so the max length of the non-terminated string is
 either 1023 or PATH_MAX - 1.
 
 Reported-by: Kevin Wolf kw...@redhat.com
 Signed-off-by: Jeff Cody jc...@redhat.com

Reviewed-by: Kevin Wolf kw...@redhat.com



[Qemu-devel] [PULL 12/16] smbios: Fix dimm size calculation when RAM is multiple of 16GB

2015-01-27 Thread Michael S. Tsirkin
From: Eduardo Habkost ehabk...@redhat.com

The Memory Device size calculation logic is broken when the RAM size is
a multiple of 16GB, making the size of the last entry be 0 instead of
16GB. Fix the logic to handle that case correctly.

Signed-off-by: Eduardo Habkost ehabk...@redhat.com
Reviewed-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
---
 hw/i386/smbios.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 024e594..ae7032a 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -850,7 +850,8 @@ void smbios_get_tables(uint8_t **tables, size_t *tables_len,
 }
 
 #define MAX_DIMM_SZ (16ll * ONE_GB)
-#define GET_DIMM_SZ ((i  dimm_cnt - 1) ? MAX_DIMM_SZ : ram_size % MAX_DIMM_SZ)
+#define GET_DIMM_SZ ((i  dimm_cnt - 1) ? MAX_DIMM_SZ \
+: ((ram_size - 1) % MAX_DIMM_SZ) + 1)
 
 dimm_cnt = QEMU_ALIGN_UP(ram_size, MAX_DIMM_SZ) / MAX_DIMM_SZ;
 
-- 
MST




Re: [Qemu-devel] [PATCH v2 21/47] acpi: add acpi_resource_template() helper

2015-01-27 Thread Michael S. Tsirkin
On Tue, Jan 27, 2015 at 02:26:34PM +0100, Claudio Fontana wrote:
 Hello Igor,
 
 On 22.01.2015 15:50, Igor Mammedov wrote:
  Signed-off-by: Igor Mammedov imamm...@redhat.com
  ---
   hw/acpi/acpi-build-utils.c | 8 
   include/hw/acpi/acpi-build-utils.h | 1 +
   2 files changed, 9 insertions(+)
  
  diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c
  index 2d5e77a..32a4377 100644
  --- a/hw/acpi/acpi-build-utils.c
  +++ b/hw/acpi/acpi-build-utils.c
  @@ -493,6 +493,14 @@ AcpiAml GCC_FMT_ATTR(1, 2) acpi_device(const char 
  *name_format, ...)
   return var;
   }
  
 
  +/* ResourceTemplate marcos helper */
 
 Since you have been so careful about putting references to the spec 
 everywhere else,
 what about adding something for ResourceTemplate macros too (note typo above)?
 
 For example 19.2.3 ASL Resource Templates if that's the right one. (I am 
 looking at version 5.1)

Pls don't, pls refer to the oldest spec that has support
for a given feature, not the newest one.

 Thanks,
 
 Claudio
 
  +AcpiAml acpi_resource_template(void)
  +{
  +/* ResourceTemplate is a buffer of Resources with EndTag at the end */
  +AcpiAml var = aml_allocate_internal(0x11 /* BufferOp */, RES_TEMPLATE);
  +return var;
  +}
  +
   /* ACPI 5.0: 20.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
   AcpiAml acpi_buffer(void)
   {
  diff --git a/include/hw/acpi/acpi-build-utils.h 
  b/include/hw/acpi/acpi-build-utils.h
  index a79c085..594fae7 100644
  --- a/include/hw/acpi/acpi-build-utils.h
  +++ b/include/hw/acpi/acpi-build-utils.h
  @@ -46,6 +46,7 @@ AcpiAml acpi_method(const char *name, int arg_count);
   AcpiAml GCC_FMT_ATTR(1, 2) acpi_scope(const char *name_format, ...);
   AcpiAml GCC_FMT_ATTR(1, 2) acpi_device(const char *name_format, ...);
   AcpiAml acpi_buffer(void);
  +AcpiAml acpi_resource_template(void);
   AcpiAml acpi_package(uint8_t num_elements);
   
   /* other helpers */
  
 
 



Re: [Qemu-devel] [PATCH] block: Simplify a few g_try_malloc() error checks

2015-01-27 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 On 27/01/2015 13:25, Markus Armbruster wrote:
 Unlike malloc(), g_try_malloc()  friends return a null pointer only
 on failure, never for a zero size.  Simplify tests for failure
 accordingly.  This helps Coverity see returned null pointers can't be
 dereferenced.  Also makes the code easier to read.

 Unfortunately that's not what I see from the source:

 gpointer
 g_try_malloc (gsize n_bytes)
 {
   gpointer mem;

   if (G_LIKELY (n_bytes))
 mem = glib_mem_vtable.try_malloc (n_bytes);
   else
 mem = NULL;

   TRACE (GLIB_MEM_ALLOC((void*) mem, (unsigned int) n_bytes, 0, 1));

   return mem;
 }

You're right.  Brain fart, please ignore.



[Qemu-devel] [Bug 897750] Re: libvirt/kvm problem with disk attach/detach/reattach on running virt

2015-01-27 Thread Stefan Bader
Serge, Scott, somehow I think we all left here in a confused state and I
do not think we still have the problem (at least not Trusty). From my
last comment it seems I did not even find something that I could change
for Precise. For now I would unassign myself and maybe this report
should be closed if Justin has no objections.

** Changed in: qemu-kvm (Ubuntu)
 Assignee: Stefan Bader (smb) = (unassigned)

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

Title:
  libvirt/kvm problem with disk attach/detach/reattach on running virt

Status in QEMU:
  New
Status in qemu-kvm package in Ubuntu:
  Confirmed

Bug description:
  Release: Ubuntu 11.10 (Oneiric)
  libvirt-bin:  0.9.2-4ubuntu15.1
  qemu-kvm:   0.14.1+noroms-0ubuntu6

  Summary:With a running KVM virt,   performing an 'attach-disk',  then a 
'detach-disk', then another 'attach-disk' 
  in an attempt to reattach the volume at the same point on the virt, fails, 
with the qemu reporting back to
  libvirt a 'Duplicate ID' error.

  Expected behavior:   The 2nd invocation of 'attach-disk' should have succeeded
  Actual behavior: Duplicate ID error reported

  
  I believe this is most likely a qemu-kvm issue, as the DOM  kvm spits back at 
libvirt after the 'detach-disk'
  does not show the just-detached disk.   There is some kind of registry/lookup 
for devices in qemu-kvm
  and for whatever reason, the entry for the disk does not get removed when it 
is detached from the
  virt.   Specifically, the error gets reported at the 2nd attach-disk attempt 
from:

qemu-option.c:qemu_opts_create:697

  684 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int 
fail_if_exists)  
  685 { 
  
  686 QemuOpts *opts = NULL;
  
  687   
  
  688 if (id) { 
  
  689 if (!id_wellformed(id)) { 
  
  690 qerror_report(QERR_INVALID_PARAMETER_VALUE, id, an 
identifier); 
  691 error_printf_unless_qmp(Identifiers consist of letters, 
digits, '-', '.', '_', starting with a letter.\n);
  692 return NULL;  
  
  693 } 
  
  694 opts = qemu_opts_find(list, id);  
  
  695 if (opts != NULL) {   
  
  696 if (fail_if_exists) { 
  
  697 qerror_report(QERR_DUPLICATE_ID, id, list-name); 
   == HERE ===  

  698 return NULL;  
  
  699 } else {  
  
  700 return opts;  
  
  701 } 
  
  702 } 
  
  703 } 
  
  704 opts = qemu_mallocz(sizeof(*opts));   
  
  705 if (id) { 
  
  706 opts-id = qemu_strdup(id);   
  
  707 } 

[Qemu-devel] [PULL 09/16] virtio: fix feature bit checks

2015-01-27 Thread Michael S. Tsirkin
From: Cornelia Huck cornelia.h...@de.ibm.com

Several places check against the feature bit number instead of against
the feature bit. Fix them.

Cc: qemu-sta...@nongnu.org
Reported-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Reviewed-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/scsi/virtio-scsi.c   | 2 +-
 hw/virtio/dataplane/vring.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index b06dd39..9e2c718 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -144,7 +144,7 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req,
  *
  * TODO: always disable this workaround for virtio 1.0 devices.
  */
-if ((vdev-guest_features  VIRTIO_F_ANY_LAYOUT) == 0) {
+if ((vdev-guest_features  (1  VIRTIO_F_ANY_LAYOUT)) == 0) {
 req_size = req-elem.out_sg[0].iov_len;
 resp_size = req-elem.in_sg[0].iov_len;
 }
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 61f6d83..78c6f45 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -133,12 +133,12 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring)
  * interrupts. */
 smp_mb();
 
-if ((vdev-guest_features  VIRTIO_F_NOTIFY_ON_EMPTY) 
+if ((vdev-guest_features  (1  VIRTIO_F_NOTIFY_ON_EMPTY)) 
 unlikely(vring-vr.avail-idx == vring-last_avail_idx)) {
 return true;
 }
 
-if (!(vdev-guest_features  VIRTIO_RING_F_EVENT_IDX)) {
+if (!(vdev-guest_features  (1  VIRTIO_RING_F_EVENT_IDX))) {
 return !(vring-vr.avail-flags  VRING_AVAIL_F_NO_INTERRUPT);
 }
 old = vring-signalled_used;
-- 
MST




[Qemu-devel] [PATCH 1/7] block/raw-posix: create translate_err helper to merge errno values

2015-01-27 Thread Denis V. Lunev
actually the code
if (ret == -ENODEV || ret == -ENOSYS || ret == -EOPNOTSUPP ||
ret == -ENOTTY) {
ret = -ENOTSUP;
}
is present twice and will be added a couple more times. Create helper
for this.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
 block/raw-posix.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e51293a..24300d0 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -893,6 +893,15 @@ static int xfs_discard(BDRVRawState *s, int64_t offset, 
uint64_t bytes)
 }
 #endif
 
+static int translate_err(int err)
+{
+if (err == -ENODEV || err == -ENOSYS || err == -EOPNOTSUPP ||
+err == -ENOTTY) {
+err = -ENOTSUP;
+}
+return err;
+}
+
 static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
 {
 int ret = -EOPNOTSUPP;
@@ -921,10 +930,9 @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData 
*aiocb)
 #endif
 }
 
-if (ret == -ENODEV || ret == -ENOSYS || ret == -EOPNOTSUPP ||
-ret == -ENOTTY) {
+ret = translate_err(ret);
+if (ret == -ENOTSUP) {
 s-has_write_zeroes = false;
-ret = -ENOTSUP;
 }
 return ret;
 }
@@ -968,10 +976,9 @@ static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb)
 #endif
 }
 
-if (ret == -ENODEV || ret == -ENOSYS || ret == -EOPNOTSUPP ||
-ret == -ENOTTY) {
+ret = translate_err(ret);
+if (ret == -ENOTSUP) {
 s-has_discard = false;
-ret = -ENOTSUP;
 }
 return ret;
 }
-- 
1.9.1




[Qemu-devel] [PATCH 3/7] block/raw-posix: refactor handle_aiocb_write_zeroes a bit

2015-01-27 Thread Denis V. Lunev
move code dealing with a block device to a separate function. This will
allow to implement additional processing for ordinary files.

Pls note, that xfs_code has been moved before checking for
s-has_write_zeroes as xfs_write_zeroes does not touch this flag inside.
This makes code a bit more consistent.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
 block/raw-posix.c | 48 +---
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 2aa268a..24e1fab 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -914,41 +914,51 @@ static int do_fallocate(int fd, int mode, off_t offset, 
off_t len)
 }
 #endif
 
-static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
+static ssize_t handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb)
 {
-int ret = -EOPNOTSUPP;
+int ret = -ENOTSUP;
 BDRVRawState *s = aiocb-bs-opaque;
 
-if (s-has_write_zeroes == 0) {
+if (!s-has_write_zeroes) {
 return -ENOTSUP;
 }
 
-if (aiocb-aio_type  QEMU_AIO_BLKDEV) {
 #ifdef BLKZEROOUT
-do {
-uint64_t range[2] = { aiocb-aio_offset, aiocb-aio_nbytes };
-if (ioctl(aiocb-aio_fildes, BLKZEROOUT, range) == 0) {
-return 0;
-}
-} while (errno == EINTR);
-
-ret = -errno;
-#endif
-} else {
-#ifdef CONFIG_XFS
-if (s-is_xfs) {
-return xfs_write_zeroes(s, aiocb-aio_offset, aiocb-aio_nbytes);
+do {
+uint64_t range[2] = { aiocb-aio_offset, aiocb-aio_nbytes };
+if (ioctl(aiocb-aio_fildes, BLKZEROOUT, range) == 0) {
+return 0;
 }
+} while (errno == EINTR);
+
+ret = translate_err(-errno);
 #endif
-}
 
-ret = translate_err(ret);
 if (ret == -ENOTSUP) {
 s-has_write_zeroes = false;
 }
 return ret;
 }
 
+static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
+{
+int ret = -ENOTSUP;
+BDRVRawState *s = aiocb-bs-opaque;
+
+if (aiocb-aio_type  QEMU_AIO_BLKDEV) {
+return handle_aiocb_write_zeroes_block(aiocb);
+}
+
+#ifdef CONFIG_XFS
+if (s-is_xfs) {
+return xfs_write_zeroes(s, aiocb-aio_offset, aiocb-aio_nbytes);
+}
+#endif
+
+s-has_write_zeroes = false;
+return ret;
+}
+
 static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb)
 {
 int ret = -EOPNOTSUPP;
-- 
1.9.1




[Qemu-devel] [PATCH 6/7] block/raw-posix: call plain fallocate in handle_aiocb_write_zeroes

2015-01-27 Thread Denis V. Lunev
There is a possibility that we are extending our image and thus writing
zeroes beyond the end of the file. In this case we do not need to care
about the hole to make sure that there is no data in the file under
this offset (pre-condition to fallocate(0) to work). We could simply call
fallocate(0).

This improves the performance of writing zeroes even on really old
platforms which do not have even FALLOC_FL_PUNCH_HOLE.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
 block/raw-posix.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c039bef..fa05239 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -60,7 +60,7 @@
 #define FS_NOCOW_FL 0x0080 /* Do not cow file */
 #endif
 #endif
-#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || 
defined(CONFIG_FALLOCATE_ZERO_RANGE)
+#ifdef CONFIG_FALLOCATE
 #include linux/falloc.h
 #endif
 #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
@@ -902,7 +902,7 @@ static int translate_err(int err)
 return err;
 }
 
-#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || 
defined(CONFIG_FALLOCATE_ZERO_RANGE)
+#ifdef CONFIG_FALLOCATE
 static int do_fallocate(int fd, int mode, off_t offset, off_t len)
 {
 do {
@@ -981,6 +981,12 @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData 
*aiocb)
 }
 #endif
 
+#ifdef CONFIG_FALLOCATE
+if (aiocb-aio_offset = aiocb-bs-total_sectors  BDRV_SECTOR_BITS) {
+return do_fallocate(s-fd, 0, aiocb-aio_offset, aiocb-aio_nbytes);
+}
+#endif
+
 s-has_write_zeroes = false;
 return ret;
 }
-- 
1.9.1




[Qemu-devel] [PULL 00/16] pci, pc, virtio fixes and cleanups

2015-01-27 Thread Michael S. Tsirkin
The following changes since commit 1e42c353469cb58ca4f3b450eea4211af7d0b147:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20150116' 
into staging (2015-01-16 12:06:41 +)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 37153450436f58449ce7e41d13a23821611e889e:

  pc-dimm: Add Error argument to pc_existing_dimms_capacity (2015-01-27 
14:46:18 +0200)


pci, pc, virtio fixes and cleanups

A bunch of fixes all over the place.  Also, beginning to generalize acpi build
code for reuse by ARM.

Signed-off-by: Michael S. Tsirkin m...@redhat.com


Alexander Graf (1):
  pci: Split pcie_host_mmcfg_map()

Amit Shah (1):
  ich9: add disable_s3, disable_s4, s4_val properties

Bharata B Rao (3):
  pc: Fix DIMMs capacity calculation
  pc-dimm: Make pc_existing_dimms_capacity global
  pc-dimm: Add Error argument to pc_existing_dimms_capacity

Cornelia Huck (1):
  virtio: fix feature bit checks

Don Koch (1):
  Add some trace calls to pci.c.

Eduardo Habkost (2):
  smbios: Fix dimm size calculation when RAM is multiple of 16GB
  smbios: Don't report unknown CPU speed (fix SVVP regression)

Igor Mammedov (2):
  pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled
  acpi: build_append_nameseg(): add padding if necessary

Michael S. Tsirkin (4):
  acpi-test: update expected DSDT
  acpi: update generated hex files
  bios-linker-loader: move header to common location
  bios-linker-loader: move source to common location

Paolo Bonzini (1):
  bios-tables-test: split piix4 and q35 tests

 {hw/i386 = include/hw/acpi}/bios-linker-loader.h |   0
 include/hw/acpi/ich9.h|   4 +
 include/hw/mem/pc-dimm.h  |   1 +
 include/hw/pci/pcie_host.h|   1 +
 hw/{i386 = acpi}/bios-linker-loader.c|   2 +-
 hw/acpi/ich9.c|  98 +-
 hw/i386/acpi-build.c  |  17 ++--
 hw/i386/pc.c  |  40 ++---
 hw/i386/smbios.c  |   8 +-
 hw/mem/pc-dimm.c  |  37 
 hw/pci/pci.c  |   9 ++
 hw/pci/pcie_host.c|   9 +-
 hw/scsi/virtio-scsi.c |   2 +-
 hw/virtio/dataplane/vring.c   |   4 +-
 tests/bios-tables-test.c  |  10 ++-
 hw/acpi/Makefile.objs |   1 +
 hw/i386/Makefile.objs |   1 -
 hw/i386/acpi-dsdt-cpu-hotplug.dsl |   1 +
 hw/i386/acpi-dsdt.hex.generated   |  43 --
 hw/i386/q35-acpi-dsdt.hex.generated   |  45 --
 tests/acpi-test-data/pc/DSDT  | Bin 3592 - 3621 bytes
 tests/acpi-test-data/q35/DSDT | Bin 8182 - 8211 bytes
 tests/acpi-test-data/q35/SSDT | Bin 560 - 560 bytes
 trace-events  |   4 +
 24 files changed, 268 insertions(+), 69 deletions(-)
 rename {hw/i386 = include/hw/acpi}/bios-linker-loader.h (100%)
 rename hw/{i386 = acpi}/bios-linker-loader.c (99%)




[Qemu-devel] [PULL 11/16] bios-linker-loader: move source to common location

2015-01-27 Thread Michael S. Tsirkin
There are plans to use bios linker by MIPS, ARM.

It's only used by ACPI ATM, so put it in hw/acpi
and make it depend on CONFIG_ACPI.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/{i386 = acpi}/bios-linker-loader.c | 0
 hw/acpi/Makefile.objs  | 1 +
 hw/i386/Makefile.objs  | 1 -
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename hw/{i386 = acpi}/bios-linker-loader.c (100%)

diff --git a/hw/i386/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
similarity index 100%
rename from hw/i386/bios-linker-loader.c
rename to hw/acpi/bios-linker-loader.c
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index acd2389..ee82073 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,3 +1,4 @@
 common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
 common-obj-$(CONFIG_ACPI) += memory_hotplug.o
 common-obj-$(CONFIG_ACPI) += acpi_interface.o
+common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 9d419ad..2b678ef 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -7,7 +7,6 @@ obj-$(CONFIG_XEN) += ../xenpv/ xen/
 
 obj-y += kvmvapic.o
 obj-y += acpi-build.o
-obj-y += bios-linker-loader.o
 hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
-- 
MST




Re: [Qemu-devel] Qemu with GDB - Query

2015-01-27 Thread Maciej W. Rozycki
On Sat, 24 Jan 2015, manish tiwari wrote:

 I am new to QEMU and trying to attach gdb with qemu on powepc host.
 
 I have tried below options
 
 qemu-system-ppc -enable-kvm -nographic -m 512 -M ppce500 -cpu e500mc -gdb
 tcp::1234 -s -S -kernel uImage -initrd rootfs.ext2.gz -append
 root=/dev/ram rw console=ttyS0,115200 -serial tcp::,server,
 
 
 With the above command I have attached gdb with qemu.
 Now I am not sure what needs to be done on host side(I have cross compiled
 GDB in the Host file system).
 
 Please help me what i need to do in host side ?

 So far you only enabled QEMU's RSP stub so that you can attach with GDB 
later on (RSP stands for GDB's Remote Serial Protocol).  So to attach from 
GDB you now need to issue a command like:

(gdb) target remote system_running_qemu:1234

from the machine you have GDB on.  Substitute `system_running_qemu' above 
with the name of the machine you have started QEMU on; if it is the same 
machine as one that runs GDB, then you can omit this part altogether.

 For the record, this is a QEMU developers' list, such questions are best 
directed to a place dedicated to QEMU users.  I am sure your favourite 
Internet search engine will be able to locate such a place for you.  
Please post your any further questions there rather than here.  Thank you.

  Maciej



Re: [Qemu-devel] [PATCH 09/50] block: Move guest_block_size into BlockBackend

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 guest_block_size is a guest device property so it should be moved into
 the interface between block layer and guest devices, which is the
 BlockBackend.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block.c   | 7 ---
  block/block-backend.c | 7 +--
  include/block/block.h | 1 -
  include/block/block_int.h | 3 ---
  4 files changed, 5 insertions(+), 13 deletions(-)
 

Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags

2015-01-27 Thread Peter Maydell
On 23 January 2015 at 18:20, Peter Maydell peter.mayd...@linaro.org wrote:
 +/* Determine the current mmu_idx to use for normal loads/stores */
  static inline int cpu_mmu_index (CPUARMState *env)
  {
 -return arm_current_el(env);
 +int el = arm_current_el(env);
 +
 +if (el  3  arm_is_secure_below_el3(env)) {
 +return ARMMMUIdx_S1SE0 + el;
 +}
 +return el;
  }

Just noticed the stray extra space between function name and '('
here; will delete the space in v2, since we're rewriting the whole
function body anyhow...

-- PMM



Re: [Qemu-devel] [PATCH 09/50] block: Move guest_block_size into BlockBackend

2015-01-27 Thread Max Reitz

On 2015-01-27 at 14:37, John Priddy wrote:

I think I mistakenly got put on the CC list for this.  Can the next replyer 
please remove j...@redhat.com?  Thanks.


Oh, sorry, I meant to CC a different John for whom I had no alias yet.

I am sorry for the spam. :-/

Max



[Qemu-devel] [PATCH RESEND 06/50] block: Add blk_is_available()

2015-01-27 Thread Max Reitz
blk_is_available() returns true iff the BDS is inserted (which means
blk_bs() is not NULL and bdrv_is_inserted() returns true) and if the
tray of the guest device is closed.

blk_is_inserted() is changed to return true only if blk_bs() is not
NULL.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/block-backend.c  | 7 ++-
 include/sysemu/block-backend.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 4c40747..4a2428e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -738,7 +738,12 @@ void blk_invalidate_cache(BlockBackend *blk, Error **errp)
 
 int blk_is_inserted(BlockBackend *blk)
 {
-return bdrv_is_inserted(blk-bs);
+return blk-bs  bdrv_is_inserted(blk-bs);
+}
+
+bool blk_is_available(BlockBackend *blk)
+{
+return blk_is_inserted(blk)  !blk_dev_is_tray_open(blk);
 }
 
 void blk_lock_medium(BlockBackend *blk, bool locked)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index f39bb5c..f91f3c7 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -127,6 +127,7 @@ int blk_enable_write_cache(BlockBackend *blk);
 void blk_set_enable_write_cache(BlockBackend *blk, bool wce);
 void blk_invalidate_cache(BlockBackend *blk, Error **errp);
 int blk_is_inserted(BlockBackend *blk);
+bool blk_is_available(BlockBackend *blk);
 void blk_lock_medium(BlockBackend *blk, bool locked);
 void blk_eject(BlockBackend *blk, bool eject_flag);
 int blk_get_flags(BlockBackend *blk);
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 04/50] hw/usb-storage: Check whether BB is inserted

2015-01-27 Thread Max Reitz
Only call bdrv_key_required() on the BlockDriverState if the
BlockBackend has an inserted medium.

Signed-off-by: Max Reitz mre...@redhat.com
---
 hw/usb/dev-storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 4539733..3123baf 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -638,7 +638,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
**errp)
 usb_msd_handle_reset(dev);
 s-scsi_dev = scsi_dev;
 
-if (bdrv_key_required(blk_bs(blk))) {
+if (blk_is_inserted(blk)  bdrv_key_required(blk_bs(blk))) {
 if (cur_mon) {
 monitor_read_bdrv_key_start(cur_mon, blk_bs(blk),
 usb_msd_password_cb, s);
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 41/50] blockdev: Add blockdev-insert-medium

2015-01-27 Thread Max Reitz
And a helper function for that which directly takes a pointer to the BDS
to be inserted instead of its node-name (which will be used for
implementing 'change' using blockdev-insert-medium).

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c   | 43 +++
 qapi/block-core.json | 17 +
 qmp-commands.hx  | 38 ++
 3 files changed, 98 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 17785b8..e4588b3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2088,6 +2088,49 @@ void qmp_blockdev_remove_medium(const char *device, 
Error **errp)
 }
 }
 
+static void qmp_blockdev_insert_anon_medium(const char *device,
+BlockDriverState *bs, Error **errp)
+{
+BlockBackend *blk;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+if (!blk_dev_has_removable_media(blk)) {
+error_setg(errp, Device '%s' is not removable, device);
+return;
+}
+
+if (!blk_dev_is_tray_open(blk)) {
+error_setg(errp, Tray of device '%s' is not open, device);
+return;
+}
+
+if (blk_bs(blk)) {
+error_setg(errp, There already is a medium in device '%s', device);
+return;
+}
+
+blk_insert_bs(blk, bs);
+}
+
+void qmp_blockdev_insert_medium(const char *device, const char *node_name,
+Error **errp)
+{
+BlockDriverState *bs;
+
+bs = bdrv_find_node(node_name);
+if (!bs) {
+error_setg(errp, Node '%s' not found, node_name);
+return;
+}
+
+qmp_blockdev_insert_anon_medium(device, bs, errp);
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1ace69d..ba41015 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1766,6 +1766,23 @@
 { 'command': 'blockdev-remove-medium',
   'data': { 'device': 'str' } }
 
+##
+# @blockdev-insert-medium:
+#
+# Inserts a medium (a block driver state tree) into a block device. That block
+# device's tray must currently be open and there must be no medium inserted
+# already.
+#
+# @device:block device name
+#
+# @node-name: name of a node in the block driver state graph
+#
+# Since: 2.3
+##
+{ 'command': 'blockdev-insert-medium',
+  'data': { 'device': 'str',
+'node-name': 'str'} }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3d5c10c..604d638 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3754,6 +3754,44 @@ Example (1):
 EQMP
 
 {
+.name   = blockdev-insert-medium,
+.args_type  = device:s,node-name:s,
+.mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium,
+},
+
+SQMP
+blockdev-insert-medium
+--
+
+Inserts a medium (a block driver state tree) into a block device. That block
+device's tray must currently be open and there must be no medium inserted
+already.
+
+Arguments:
+
+- device: block device name (json-string)
+- node-name: root node of the BDS tree to insert into the block device
+
+Example (1):
+
+- { execute: blockdev-add,
+ arguments: { options: { id: backend0,
+ node-name: node0,
+ driver: raw,
+ file: { driver: file,
+   filename: fedora.iso } } } }
+
+- { return: {} }
+
+- { execute: blockdev-insert-medium,
+ arguments: { device: ide1-cd0,
+node-name: node0 } }
+
+- { return: {} }
+
+EQMP
+
+{
 .name   = query-named-block-nodes,
 .args_type  = ,
 .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes,
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 43/50] blockdev: Implement change with basic operations

2015-01-27 Thread Max Reitz
Implement 'change' on block devices by calling blockdev-open-tray,
blockdev-remove-medium, blockdev-insert-medium (a variation of that
which does not need a node-name) and blockdev-close-tray.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 191 +++--
 1 file changed, 72 insertions(+), 119 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0b204eb..6e440b8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1828,41 +1828,6 @@ exit:
 }
 }
 
-
-static void eject_device(BlockBackend *blk, int force, Error **errp)
-{
-BlockDriverState *bs = blk_bs(blk);
-AioContext *aio_context;
-
-aio_context = blk_get_aio_context(blk);
-aio_context_acquire(aio_context);
-
-if (bs  bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
-goto out;
-}
-if (!blk_dev_has_removable_media(blk)) {
-error_setg(errp, Device '%s' is not removable,
-   bdrv_get_device_name(bs));
-goto out;
-}
-
-if (blk_dev_is_medium_locked(blk)  !blk_dev_is_tray_open(blk)) {
-blk_dev_eject_request(blk, force);
-if (!force) {
-error_setg(errp, Device '%s' is locked,
-   bdrv_get_device_name(bs));
-goto out;
-}
-}
-
-if (bs) {
-bdrv_close(bs);
-}
-
-out:
-aio_context_release(aio_context);
-}
-
 void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
 {
 Error *local_err = NULL;
@@ -1909,90 +1874,6 @@ out:
 aio_context_release(aio_context);
 }
 
-/* Assumes AioContext is held */
-static void qmp_bdrv_open_encrypted(BlockDriverState **pbs,
-const char *filename,
-int bdrv_flags, BlockDriver *drv,
-const char *password, Error **errp)
-{
-BlockDriverState *bs;
-Error *local_err = NULL;
-int ret;
-
-ret = bdrv_open(pbs, filename, NULL, NULL, bdrv_flags, drv, local_err);
-if (ret  0) {
-error_propagate(errp, local_err);
-return;
-}
-bs = *pbs;
-
-if (bdrv_key_required(bs)) {
-if (password) {
-if (bdrv_set_key(bs, password)  0) {
-error_set(errp, QERR_INVALID_PASSWORD);
-}
-} else {
-error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
-  bdrv_get_encrypted_filename(bs));
-}
-} else if (password) {
-error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
-}
-}
-
-void qmp_change_blockdev(const char *device, const char *filename,
- const char *format, Error **errp)
-{
-BlockBackend *blk;
-BlockDriverState *bs;
-AioContext *aio_context;
-BlockDriver *drv = NULL;
-int bdrv_flags;
-bool new_bs;
-Error *err = NULL;
-
-blk = blk_by_name(device);
-if (!blk) {
-error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-return;
-}
-bs = blk_bs(blk);
-new_bs = !bs;
-
-aio_context = blk_get_aio_context(blk);
-aio_context_acquire(aio_context);
-
-if (format) {
-drv = bdrv_find_whitelisted_format(format, blk_is_read_only(blk));
-if (!drv) {
-error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-goto out;
-}
-}
-
-eject_device(blk, 0, err);
-if (err) {
-error_propagate(errp, err);
-goto out;
-}
-
-bdrv_flags = blk_is_read_only(blk) ? 0 : BDRV_O_RDWR;
-bdrv_flags |= blk_get_root_state(blk)-open_flags  ~BDRV_O_RDWR;
-
-qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, err);
-if (err) {
-error_propagate(errp, err);
-} else if (new_bs) {
-blk_insert_bs(blk, bs);
-/* Has been sent automatically by bdrv_open() if blk_bs(blk) was not
- * NULL */
-blk_dev_change_media_cb(blk, true);
-}
-
-out:
-aio_context_release(aio_context);
-}
-
 void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
 Error **errp)
 {
@@ -2131,6 +2012,78 @@ void qmp_blockdev_insert_medium(const char *device, 
const char *node_name,
 qmp_blockdev_insert_anon_medium(device, bs, errp);
 }
 
+void qmp_change_blockdev(const char *device, const char *filename,
+ const char *format, Error **errp)
+{
+BlockBackend *blk;
+BlockBackendRootState *blk_rs;
+BlockDriverState *medium_bs;
+BlockDriver *drv = NULL;
+int bdrv_flags, ret;
+Error *err = NULL;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+if (blk_bs(blk)) {
+blk_update_root_state(blk);
+}
+
+blk_rs = blk_get_root_state(blk);
+bdrv_flags = blk_rs-read_only ? 0 : BDRV_O_RDWR;
+bdrv_flags |= blk_rs-open_flags  ~BDRV_O_RDWR;
+
+if (format) {
+drv = 

[Qemu-devel] [PATCH RESEND 38/50] blockdev: Add blockdev-open-tray

2015-01-27 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c   | 48 
 qapi/block-core.json | 21 +
 qmp-commands.hx  | 37 +
 3 files changed, 106 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2f3ccb5..a0b9f17 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1993,6 +1993,54 @@ out:
 aio_context_release(aio_context);
 }
 
+void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
+Error **errp)
+{
+BlockBackend *blk;
+BlockDriverState *bs;
+AioContext *aio_context = NULL;
+
+if (!has_force) {
+force = false;
+}
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+if (!blk_dev_has_removable_media(blk)) {
+error_setg(errp, Device '%s' is not removable, device);
+return;
+}
+
+if (blk_dev_is_tray_open(blk)) {
+return;
+}
+
+bs = blk_bs(blk);
+if (bs) {
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+goto out;
+}
+}
+
+if (blk_dev_is_medium_locked(blk)) {
+blk_dev_eject_request(blk, force);
+} else {
+blk_dev_change_media_cb(blk, false);
+}
+
+out:
+if (aio_context) {
+aio_context_release(aio_context);
+}
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ca9bc32..029f08b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1718,6 +1718,27 @@
 ##
 { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
 
+##
+# @blockdev-open-tray:
+#
+# Opens a block device's tray. If there is a block driver state tree inserted 
as
+# a medium, it will become inaccessible to the guest (but it will remain
+# associated to the block device, so closing the tray will make it accessible
+# again).
+#
+# @device: block device name
+#
+# @force:  #optional if false (the default), an eject request will be sent to
+#  the guest if it has locked the tray (and the tray will not be opened
+#  immediately); if true, the tray will be opened regardless of whether
+#  it is locked
+#
+# Since: 2.3
+##
+{ 'command': 'blockdev-open-tray',
+  'data': { 'device': 'str',
+'*force': 'bool' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7576c18..cfa1b98 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3641,6 +3641,43 @@ Example (2):
 EQMP
 
 {
+.name   = blockdev-open-tray,
+.args_type  = device:s,force:b?,
+.mhandler.cmd_new = qmp_marshal_input_blockdev_open_tray,
+},
+
+SQMP
+blockdev-open-tray
+--
+
+Opens a block device's tray. If there is a block driver state tree inserted as 
a
+medium, it will become inaccessible to the guest (but it will remain associated
+to the block device, so closing the tray will make it accessible again).
+
+Arguments:
+
+- device: block device name (json-string)
+- force: if false (the default), an eject request will be sent to the guest 
if
+   it has locked the tray (and the tray will not be opened 
immediately);
+   if true, the tray will be opened regardless of whether it is locked
+   (json-bool, optional)
+
+Example (1):
+
+- { execute: blockdev-open-tray,
+ arguments: { device: ide1-cd0 } }
+
+- { timestamp: { seconds: 1418751016,
+microseconds: 716996 },
+ event: DEVICE_TRAY_MOVED,
+ data: { device: ide1-cd0,
+   tray-open: true } }
+
+- { return: {} }
+
+EQMP
+
+{
 .name   = query-named-block-nodes,
 .args_type  = ,
 .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes,
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 44/50] block: Inquire tray state before tray-moved events

2015-01-27 Thread Max Reitz
blk_dev_change_media_cb() is called for all potential tray movements;
however, it is possible to request closing the tray but nothing actually
happening (on a floppy disk drive without a medium).

Thus, the actual tray status should be inquired before sending a
tray-moved event (and an event should be sent whenever the status
changed).

Checking @load is now superfluous; it was necessary because it was
possible to change a medium without having explicitly opened the tray
and closed it again (or it might have been possible, at least). This is
no longer possible, though.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/block-backend.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index d2c1bff..7c18f8d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -420,18 +420,15 @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps 
*ops,
 void blk_dev_change_media_cb(BlockBackend *blk, bool load)
 {
 if (blk-dev_ops  blk-dev_ops-change_media_cb) {
-bool tray_was_closed = !blk_dev_is_tray_open(blk);
+bool tray_was_open, tray_is_open;
 
+tray_was_open = blk_dev_is_tray_open(blk);
 blk-dev_ops-change_media_cb(blk-dev_opaque, load);
-if (tray_was_closed) {
-/* tray open */
-qapi_event_send_device_tray_moved(blk_name(blk),
-  true, error_abort);
-}
-if (load) {
-/* tray close */
-qapi_event_send_device_tray_moved(blk_name(blk),
-  false, error_abort);
+tray_is_open = blk_dev_is_tray_open(blk);
+
+if (tray_was_open != tray_is_open) {
+qapi_event_send_device_tray_moved(blk_name(blk), tray_is_open,
+  error_abort);
 }
 }
 }
-- 
2.1.0




Re: [Qemu-devel] [PATCH 03/50] hw/block/fdc: Implement tray status

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 The tray of an FDD is open iff there is no medium inserted (there are
 only two states for an FDD: medium inserted or no medium inserted).
 
 This results in the tray being reported as open if qemu has been started
 with the default floppy drive, which breaks some tests. Fix them.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  hw/block/fdc.c | 20 +---
  tests/fdc-test.c   |  4 +---
  tests/qemu-iotests/067.out | 60 
 +++---
  tests/qemu-iotests/071.out |  2 --
  tests/qemu-iotests/081.out |  1 -
  tests/qemu-iotests/087.out |  5 
  6 files changed, 26 insertions(+), 66 deletions(-)
 

 +++ b/hw/block/fdc.c
 @@ -192,6 +192,8 @@ typedef struct FDrive {
  uint8_t ro;   /* Is read-only   */
  uint8_t media_changed;/* Is media changed   */

Might be nice someday to make these bool, but not in this patch.

  uint8_t media_rate;   /* Data rate of medium*/
 +
 +bool media_inserted;  /* Is there a medium in the tray */
  } FDrive;
  

Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH RESEND 12/50] block: Move I/O status and error actions into BB

2015-01-27 Thread Max Reitz
These options are only relevant for the user of a whole BDS tree (like a
guest device or a block job) and should thus be moved into the
BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block.c| 125 -
 block/backup.c |  17 --
 block/block-backend.c  | 116 --
 block/commit.c |   3 +-
 block/mirror.c |  17 --
 block/qapi.c   |   4 +-
 block/stream.c |   3 +-
 blockdev.c |   6 +-
 blockjob.c |   5 +-
 include/block/block.h  |  11 
 include/block/block_int.h  |   6 --
 include/sysemu/block-backend.h |   7 +++
 qmp.c  |   6 +-
 13 files changed, 158 insertions(+), 168 deletions(-)

diff --git a/block.c b/block.c
index 17e4ee3..9a0a510 100644
--- a/block.c
+++ b/block.c
@@ -371,7 +371,6 @@ BlockDriverState *bdrv_new(void)
 for (i = 0; i  BLOCK_OP_TYPE_MAX; i++) {
 QLIST_INIT(bs-op_blockers[i]);
 }
-bdrv_iostatus_disable(bs);
 notifier_list_init(bs-close_notifiers);
 notifier_with_return_list_init(bs-before_write_notifiers);
 qemu_co_queue_init(bs-throttled_reqs[0]);
@@ -2050,14 +2049,6 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 bs_dest-throttled_reqs[1]  = bs_src-throttled_reqs[1];
 bs_dest-io_limits_enabled  = bs_src-io_limits_enabled;
 
-/* r/w error */
-bs_dest-on_read_error  = bs_src-on_read_error;
-bs_dest-on_write_error = bs_src-on_write_error;
-
-/* i/o status */
-bs_dest-iostatus_enabled   = bs_src-iostatus_enabled;
-bs_dest-iostatus   = bs_src-iostatus;
-
 /* dirty bitmap */
 bs_dest-dirty_bitmaps  = bs_src-dirty_bitmaps;
 
@@ -3562,82 +3553,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t 
*nb_sectors_ptr)
 *nb_sectors_ptr = nb_sectors  0 ? 0 : nb_sectors;
 }
 
-void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
-   BlockdevOnError on_write_error)
-{
-bs-on_read_error = on_read_error;
-bs-on_write_error = on_write_error;
-}
-
-BlockdevOnError bdrv_get_on_error(BlockDriverState *bs, bool is_read)
-{
-return is_read ? bs-on_read_error : bs-on_write_error;
-}
-
-BlockErrorAction bdrv_get_error_action(BlockDriverState *bs, bool is_read, int 
error)
-{
-BlockdevOnError on_err = is_read ? bs-on_read_error : bs-on_write_error;
-
-switch (on_err) {
-case BLOCKDEV_ON_ERROR_ENOSPC:
-return (error == ENOSPC) ?
-   BLOCK_ERROR_ACTION_STOP : BLOCK_ERROR_ACTION_REPORT;
-case BLOCKDEV_ON_ERROR_STOP:
-return BLOCK_ERROR_ACTION_STOP;
-case BLOCKDEV_ON_ERROR_REPORT:
-return BLOCK_ERROR_ACTION_REPORT;
-case BLOCKDEV_ON_ERROR_IGNORE:
-return BLOCK_ERROR_ACTION_IGNORE;
-default:
-abort();
-}
-}
-
-static void send_qmp_error_event(BlockDriverState *bs,
- BlockErrorAction action,
- bool is_read, int error)
-{
-IoOperationType optype;
-
-optype = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE;
-qapi_event_send_block_io_error(bdrv_get_device_name(bs), optype, action,
-   bdrv_iostatus_is_enabled(bs),
-   error == ENOSPC, strerror(error),
-   error_abort);
-}
-
-/* This is done by device models because, while the block layer knows
- * about the error, it does not know whether an operation comes from
- * the device or the block layer (from a job, for example).
- */
-void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
-   bool is_read, int error)
-{
-assert(error = 0);
-
-if (action == BLOCK_ERROR_ACTION_STOP) {
-/* First set the iostatus, so that info block returns an iostatus
- * that matches the events raised so far (an additional error iostatus
- * is fine, but not a lost one).
- */
-bdrv_iostatus_set_err(bs, error);
-
-/* Then raise the request to stop the VM and the event.
- * qemu_system_vmstop_request_prepare has two effects.  First,
- * it ensures that the STOP event always comes after the
- * BLOCK_IO_ERROR event.  Second, it ensures that even if management
- * can observe the STOP event and do a cont before the STOP
- * event is issued, the VM will not stop.  In this case, vm_start()
- * also ensures that the STOP/RESUME pair of events is emitted.
- */
-qemu_system_vmstop_request_prepare();
-send_qmp_error_event(bs, action, is_read, error);
-qemu_system_vmstop_request(RUN_STATE_IO_ERROR);
-} else {
-send_qmp_error_event(bs, action, is_read, error);
-}
-}
-
 int bdrv_is_read_only(BlockDriverState *bs)

[Qemu-devel] [PATCH RESEND 50/50] iotests: Add test for change-related QMP commands

2015-01-27 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
---
 tests/qemu-iotests/118 | 649 +
 tests/qemu-iotests/118.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 655 insertions(+)
 create mode 100755 tests/qemu-iotests/118
 create mode 100644 tests/qemu-iotests/118.out

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
new file mode 100755
index 000..06140e9
--- /dev/null
+++ b/tests/qemu-iotests/118
@@ -0,0 +1,649 @@
+#!/usr/bin/env python
+#
+# Test case for the QMP 'change' command and all other associated
+# commands
+#
+# Copyright (C) 2015 Red Hat, Inc.
+#
+# This program 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 program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+#
+
+import os
+import stat
+import iotests
+from iotests import qemu_img
+
+old_img = os.path.join(iotests.test_dir, 'test0.img')
+new_img = os.path.join(iotests.test_dir, 'test1.img')
+
+class ChangeBaseClass(iotests.QMPTestCase):
+has_opened = False
+has_closed = False
+
+def process_events(self):
+for event in self.vm.get_qmp_events(wait=True):
+if event['event'] == 'DEVICE_TRAY_MOVED' and 
event['data']['device'] == 'drive0':
+if event['data']['tray-open'] == False:
+self.has_closed = True
+else:
+self.has_opened = True
+
+class GeneralChangeTestsBaseClass(ChangeBaseClass):
+def test_change(self):
+result = self.vm.qmp('change', device='drive0', target=new_img,
+   arg=iotests.imgfmt)
+self.assert_qmp(result, 'return', {})
+
+while not self.has_opened:
+self.process_events()
+while not self.has_closed:
+self.process_events()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', False)
+self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+def test_blockdev_change_medium(self):
+result = self.vm.qmp('blockdev-change-medium', device='drive0',
+   filename=new_img,
+   format=iotests.imgfmt)
+self.assert_qmp(result, 'return', {})
+
+while not self.has_opened:
+self.process_events()
+while not self.has_closed:
+self.process_events()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', False)
+self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+def test_eject(self):
+result = self.vm.qmp('eject', device='drive0', force=True)
+self.assert_qmp(result, 'return', {})
+
+while not self.has_opened:
+self.process_events()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', True)
+self.assert_qmp_absent(result, 'return[0]/inserted')
+
+def test_tray_eject_change(self):
+result = self.vm.qmp('eject', device='drive0', force=True)
+self.assert_qmp(result, 'return', {})
+
+while not self.has_opened:
+self.process_events()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', True)
+self.assert_qmp_absent(result, 'return[0]/inserted')
+
+result = self.vm.qmp('blockdev-change-medium', device='drive0',
+   filename=new_img,
+   format=iotests.imgfmt)
+self.assert_qmp(result, 'return', {})
+
+while not self.has_closed:
+self.process_events()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', False)
+self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+def test_tray_open_close(self):
+result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True)
+self.assert_qmp(result, 'return', {})
+
+while not self.has_opened:
+self.process_events()
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/tray_open', True)
+if self.was_empty == True:
+self.assert_qmp_absent(result, 'return[0]/inserted')
+else:
+self.assert_qmp(result, 

Re: [Qemu-devel] [PATCH 09/11] target-arm: Use mmu_idx in get_phys_addr()

2015-01-27 Thread Peter Maydell
On 27 January 2015 at 17:57, Greg Bellows greg.bell...@linaro.org wrote:


 On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell peter.mayd...@linaro.org
 wrote:
 +/* Return the exception level which controls this address translation
 regime */
 +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
 +{
 +switch (mmu_idx) {
 +case ARMMMUIdx_S2NS:
 +case ARMMMUIdx_S1E2:
 +return 2;
 +case ARMMMUIdx_S1E3:
 +return 3;
 +case ARMMMUIdx_S1SE0:
 +return arm_el_is_aa64(env, 3) ? 1 : 3;
 +case ARMMMUIdx_S1SE1:


 I think this should be handled the same way as S1SE0 as Secure EL1 maps to
 EL3 when EL3 is AArch32.

If EL3 is AArch32 then you'll never be using this MMU index.
By definition the S1SE1 index is for code executing at
Secure EL1, and there isn't any of that unless EL3 is 64 bit.
(Secure EL1 doesn't map to anything, it just doesn't
exist/have any code running in it.)

 +case ARMMMUIdx_S1NSE0:
 +case ARMMMUIdx_S1NSE1:
 +return 1;
 +default:
 +g_assert_not_reached();
 +}
 +}
 +
 +/* Return the SCTLR value which controls this address translation regime
 */
 +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
 +{
 +return env-cp15.sctlr_el[regime_el(env, mmu_idx)];


 Given the above regime_el(), for S1SE1, this would return the non-secure
 SCTLR bank on a secure translation.  Same below for TCR and all uses
 thereafter.

That's correct, because S1SE1 implies secure EL1 under a 64 bit
EL3, in which case there is no system register banking and
both Secure and NonSecure use the same underlying register.
Compare the way that A32_BANKED_CURRENT_REG_GET/SET always
use the NS version if arm_el_is_aa64(env, 3).

 +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
 +{
 +switch (mmu_idx) {
 +case ARMMMUIdx_S1SE0:
 +case ARMMMUIdx_S1NSE0:


 The get_phys_addr path filters out ARMMMUIdx_S12NSE0 so calls to this
 function in this context don't matter, but what if it is called outside this
 context?  How should it handle this index?

g_assert_not_reached(),  but it didn't seem worth cluttering
the switch with a bunch of extra labels just to assert that
they weren't reachable.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 2/2] pc: memory: Validate alignment of maxram_size to page size

2015-01-27 Thread Eric Blake
On 01/26/2015 08:31 AM, Peter Krempa wrote:
 If the maxram_size is not aligned and dimm devices were added on the
 command line qemu would terminate with a rather unhelpful message:
 
 ERROR:hw/mem/pc-dimm.c:150:pc_dimm_get_free_addr: assertion failed:
 (QEMU_ALIGN_UP(address_space_size, align) == address_space_size)
 
 In case no dimm device was originally added on the commandline qemu
 exits on the assertion failure.
 
 Signed-off-by: Peter Krempa pkre...@redhat.com
 ---
  hw/i386/pc.c | 7 +++
  1 file changed, 7 insertions(+)

Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH RESEND 05/50] block: Fix BB AIOCB AioContext without BDS

2015-01-27 Thread Max Reitz
Fix the BlockBackend's AIOCB AioContext for aborting AIO in case there
is no BDS. If there is no implementation of AIOCBInfo::get_aio_context()
the AioContext is derived from the BDS the AIOCB belongs to. If that BDS
is NULL (because it has been removed from the BB) this will not work.

This patch makes blk_get_aio_context() fall back to the main loop
context if the BDS pointer is NULL and implements
AIOCBInfo::get_aio_context() (blk_aiocb_get_aio_context()) which invokes
blk_get_aio_context().

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/block-backend.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 96a5bc6..4c40747 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -18,6 +18,8 @@
 /* Number of coroutines to reserve per attached device model */
 #define COROUTINE_POOL_RESERVATION 64
 
+static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb);
+
 struct BlockBackend {
 char *name;
 int refcnt;
@@ -34,10 +36,12 @@ struct BlockBackend {
 typedef struct BlockBackendAIOCB {
 BlockAIOCB common;
 QEMUBH *bh;
+BlockBackend *blk;
 int ret;
 } BlockBackendAIOCB;
 
 static const AIOCBInfo block_backend_aiocb_info = {
+.get_aio_context = blk_aiocb_get_aio_context,
 .aiocb_size = sizeof(BlockBackendAIOCB),
 };
 
@@ -530,6 +534,7 @@ static BlockAIOCB *abort_aio_request(BlockBackend *blk, 
BlockCompletionFunc *cb,
 QEMUBH *bh;
 
 acb = blk_aio_get(block_backend_aiocb_info, blk, cb, opaque);
+acb-blk = blk;
 acb-ret = ret;
 
 bh = aio_bh_new(blk_get_aio_context(blk), error_callback_bh, acb);
@@ -783,7 +788,17 @@ void blk_op_unblock_all(BlockBackend *blk, Error *reason)
 
 AioContext *blk_get_aio_context(BlockBackend *blk)
 {
-return bdrv_get_aio_context(blk-bs);
+if (blk-bs) {
+return bdrv_get_aio_context(blk-bs);
+} else {
+return qemu_get_aio_context();
+}
+}
+
+static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
+{
+BlockBackendAIOCB *blk_acb = DO_UPCAST(BlockBackendAIOCB, common, acb);
+return blk_get_aio_context(blk_acb-blk);
 }
 
 void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 07/50] block: Make bdrv_is_inserted() recursive

2015-01-27 Thread Max Reitz
If bdrv_is_inserted() is called on the top level BDS, it should make
sure all nodes in the BDS tree are actually inserted.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 4a5f8fc..aff7682 100644
--- a/block.c
+++ b/block.c
@@ -5208,11 +5208,12 @@ int bdrv_is_inserted(BlockDriverState *bs)
 {
 BlockDriver *drv = bs-drv;
 
-if (!drv)
+if (!drv) {
 return 0;
-if (!drv-bdrv_is_inserted)
-return 1;
-return drv-bdrv_is_inserted(bs);
+}
+return (!drv-bdrv_is_inserted || drv-bdrv_is_inserted(bs)) 
+   (!bs-file  || bdrv_is_inserted(bs-file)) 
+   (!bs-backing_hd|| bdrv_is_inserted(bs-backing_hd));
 }
 
 /**
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 11/50] block: Move BlockAcctStats into BlockBackend

2015-01-27 Thread Max Reitz
As the comment above bdrv_get_stats() says, BlockAcctStats is something
which belongs to the device instead of each BlockDriverState. This patch
therefore moves it into the BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block.c   | 11 ---
 block/block-backend.c |  5 -
 block/qapi.c  | 20 
 include/block/block.h |  2 --
 include/block/block_int.h |  3 ---
 5 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/block.c b/block.c
index 5db71c6..17e4ee3 100644
--- a/block.c
+++ b/block.c
@@ -6119,14 +6119,3 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 QDECREF(json);
 }
 }
-
-/* This accessor function purpose is to allow the device models to access the
- * BlockAcctStats structure embedded inside a BlockDriverState without being
- * aware of the BlockDriverState structure layout.
- * It will go away when the BlockAcctStats structure will be moved inside
- * the device models.
- */
-BlockAcctStats *bdrv_get_stats(BlockDriverState *bs)
-{
-return bs-stats;
-}
diff --git a/block/block-backend.c b/block/block-backend.c
index bf0fcc9..3d565d8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -34,6 +34,9 @@ struct BlockBackend {
 
 /* the block size for which the guest device expects atomicity */
 int guest_block_size;
+
+/* I/O stats (display with info blockstats). */
+BlockAcctStats stats;
 };
 
 typedef struct BlockBackendAIOCB {
@@ -849,7 +852,7 @@ void blk_io_unplug(BlockBackend *blk)
 
 BlockAcctStats *blk_get_stats(BlockBackend *blk)
 {
-return bdrv_get_stats(blk-bs);
+return blk-stats;
 }
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
diff --git a/block/qapi.c b/block/qapi.c
index 4e97574..7840c81 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -334,14 +334,18 @@ static BlockStats *bdrv_query_stats(const 
BlockDriverState *bs,
 }
 
 s-stats = g_malloc0(sizeof(*s-stats));
-s-stats-rd_bytes = bs-stats.nr_bytes[BLOCK_ACCT_READ];
-s-stats-wr_bytes = bs-stats.nr_bytes[BLOCK_ACCT_WRITE];
-s-stats-rd_operations = bs-stats.nr_ops[BLOCK_ACCT_READ];
-s-stats-wr_operations = bs-stats.nr_ops[BLOCK_ACCT_WRITE];
-s-stats-flush_operations = bs-stats.nr_ops[BLOCK_ACCT_FLUSH];
-s-stats-wr_total_time_ns = bs-stats.total_time_ns[BLOCK_ACCT_WRITE];
-s-stats-rd_total_time_ns = bs-stats.total_time_ns[BLOCK_ACCT_READ];
-s-stats-flush_total_time_ns = bs-stats.total_time_ns[BLOCK_ACCT_FLUSH];
+if (bs-blk) {
+BlockAcctStats *stats = blk_get_stats(bs-blk);
+
+s-stats-rd_bytes = stats-nr_bytes[BLOCK_ACCT_READ];
+s-stats-wr_bytes = stats-nr_bytes[BLOCK_ACCT_WRITE];
+s-stats-rd_operations = stats-nr_ops[BLOCK_ACCT_READ];
+s-stats-wr_operations = stats-nr_ops[BLOCK_ACCT_WRITE];
+s-stats-flush_operations = stats-nr_ops[BLOCK_ACCT_FLUSH];
+s-stats-wr_total_time_ns = stats-total_time_ns[BLOCK_ACCT_WRITE];
+s-stats-rd_total_time_ns = stats-total_time_ns[BLOCK_ACCT_READ];
+s-stats-flush_total_time_ns = stats-total_time_ns[BLOCK_ACCT_FLUSH];
+}
 
 s-stats-wr_highest_offset = bs-wr_highest_sector * BDRV_SECTOR_SIZE;
 
diff --git a/include/block/block.h b/include/block/block.h
index df656db..8cd6b31 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -550,6 +550,4 @@ void bdrv_io_plug(BlockDriverState *bs);
 void bdrv_io_unplug(BlockDriverState *bs);
 void bdrv_flush_io_queue(BlockDriverState *bs);
 
-BlockAcctStats *bdrv_get_stats(BlockDriverState *bs);
-
 #endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e309d8a..d023913 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -363,9 +363,6 @@ struct BlockDriverState {
 CoQueue  throttled_reqs[2];
 bool io_limits_enabled;
 
-/* I/O stats (display with info blockstats). */
-BlockAcctStats stats;
-
 /* Highest sector index written to */
 uint64_t wr_highest_sector;
 
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 09/50] block: Move guest_block_size into BlockBackend

2015-01-27 Thread Max Reitz
guest_block_size is a guest device property so it should be moved into
the interface between block layer and guest devices, which is the
BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block.c   | 7 ---
 block/block-backend.c | 7 +--
 include/block/block.h | 1 -
 include/block/block_int.h | 3 ---
 4 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index aff7682..eff92ca 100644
--- a/block.c
+++ b/block.c
@@ -965,7 +965,6 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 }
 
 bs-open_flags = flags;
-bs-guest_block_size = 512;
 bs-request_alignment = 512;
 bs-zero_beyond_eof = true;
 open_flags = bdrv_open_flags(bs, flags);
@@ -2039,7 +2038,6 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 /* move some fields that need to stay attached to the device */
 
 /* dev info */
-bs_dest-guest_block_size   = bs_src-guest_block_size;
 bs_dest-copy_on_read   = bs_src-copy_on_read;
 
 bs_dest-enable_write_cache = bs_src-enable_write_cache;
@@ -5286,11 +5284,6 @@ BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 return NULL;
 }
 
-void bdrv_set_guest_block_size(BlockDriverState *bs, int align)
-{
-bs-guest_block_size = align;
-}
-
 void *qemu_blockalign(BlockDriverState *bs, size_t size)
 {
 return qemu_memalign(bdrv_opt_mem_align(bs), size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 4a2428e..bf0fcc9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -31,6 +31,9 @@ struct BlockBackend {
 /* TODO change to DeviceState when all users are qdevified */
 const BlockDevOps *dev_ops;
 void *dev_opaque;
+
+/* the block size for which the guest device expects atomicity */
+int guest_block_size;
 };
 
 typedef struct BlockBackendAIOCB {
@@ -334,7 +337,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
 blk-dev = NULL;
 blk-dev_ops = NULL;
 blk-dev_opaque = NULL;
-bdrv_set_guest_block_size(blk-bs, 512);
+blk-guest_block_size = 512;
 blk_unref(blk);
 }
 
@@ -763,7 +766,7 @@ int blk_get_flags(BlockBackend *blk)
 
 void blk_set_guest_block_size(BlockBackend *blk, int align)
 {
-bdrv_set_guest_block_size(blk-bs, align);
+blk-guest_block_size = align;
 }
 
 void *blk_blockalign(BlockBackend *blk, size_t size)
diff --git a/include/block/block.h b/include/block/block.h
index 3082d2b..df656db 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -425,7 +425,6 @@ void bdrv_img_create(const char *filename, const char *fmt,
 /* Returns the alignment in bytes that is required so that no bounce buffer
  * is required throughout the stack */
 size_t bdrv_opt_mem_align(BlockDriverState *bs);
-void bdrv_set_guest_block_size(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
 void *qemu_blockalign0(BlockDriverState *bs, size_t size);
 void *qemu_try_blockalign(BlockDriverState *bs, size_t size);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index b340e7e..c6ab73a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -375,9 +375,6 @@ struct BlockDriverState {
 /* Alignment requirement for offset/length of I/O requests */
 unsigned int request_alignment;
 
-/* the block size for which the guest device expects atomicity */
-int guest_block_size;
-
 /* do we need to tell the quest if we have a volatile write cache? */
 int enable_write_cache;
 
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 25/50] blockdev: Check BB validity in block-commit

2015-01-27 Thread Max Reitz
Call blk_is_available() before using blk_bs() to obtain the root
BlockDriverState behind the BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 74d26a6..431afcd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2213,6 +2213,7 @@ void qmp_block_commit(const char *device,
   bool has_speed, int64_t speed,
   Error **errp)
 {
+BlockBackend *blk;
 BlockDriverState *bs;
 BlockDriverState *base_bs, *top_bs;
 AioContext *aio_context;
@@ -2231,15 +2232,21 @@ void qmp_block_commit(const char *device,
  *  live commit feature versions; for this to work, we must make sure to
  *  perform the device lookup before any generic errors that may occur in a
  *  scenario in which all optional arguments are omitted. */
-bs = bdrv_find(device);
-if (!bs) {
+blk = blk_by_name(device);
+if (!blk) {
 error_set(errp, QERR_DEVICE_NOT_FOUND, device);
 return;
 }
 
-aio_context = bdrv_get_aio_context(bs);
+aio_context = blk_get_aio_context(blk);
 aio_context_acquire(aio_context);
 
+if (!blk_is_available(blk)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+goto out;
+}
+bs = blk_bs(blk);
+
 /* drain all i/o before commits */
 bdrv_drain_all();
 
-- 
2.1.0




Re: [Qemu-devel] [PATCH 09/11] target-arm: Use mmu_idx in get_phys_addr()

2015-01-27 Thread Greg Bellows
On Tue, Jan 27, 2015 at 12:12 PM, Peter Maydell peter.mayd...@linaro.org
wrote:

 On 27 January 2015 at 17:57, Greg Bellows greg.bell...@linaro.org wrote:
 
 
  On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell 
 peter.mayd...@linaro.org
  wrote:
  +/* Return the exception level which controls this address translation
  regime */
  +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
  +{
  +switch (mmu_idx) {
  +case ARMMMUIdx_S2NS:
  +case ARMMMUIdx_S1E2:
  +return 2;
  +case ARMMMUIdx_S1E3:
  +return 3;
  +case ARMMMUIdx_S1SE0:
  +return arm_el_is_aa64(env, 3) ? 1 : 3;
  +case ARMMMUIdx_S1SE1:
 
 
  I think this should be handled the same way as S1SE0 as Secure EL1 maps
 to
  EL3 when EL3 is AArch32.

 If EL3 is AArch32 then you'll never be using this MMU index.
 By definition the S1SE1 index is for code executing at
 Secure EL1, and there isn't any of that unless EL3 is 64 bit.
 (Secure EL1 doesn't map to anything, it just doesn't
 exist/have any code running in it.)


​Ah yes and thanks for the pointer to the origin of mmu index I have now
connected where we prevent that index with this code.



  +case ARMMMUIdx_S1NSE0:
  +case ARMMMUIdx_S1NSE1:
  +return 1;
  +default:
  +g_assert_not_reached();
  +}
  +}
  +
  +/* Return the SCTLR value which controls this address translation
 regime
  */
  +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx
 mmu_idx)
  +{
  +return env-cp15.sctlr_el[regime_el(env, mmu_idx)];
 
 
  Given the above regime_el(), for S1SE1, this would return the non-secure
  SCTLR bank on a secure translation.  Same below for TCR and all uses
  thereafter.

 That's correct, because S1SE1 implies secure EL1 under a 64 bit
 EL3, in which case there is no system register banking and
 both Secure and NonSecure use the same underlying register.
 Compare the way that A32_BANKED_CURRENT_REG_GET/SET always
 use the NS version if arm_el_is_aa64(env, 3).

  +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
  +{
  +switch (mmu_idx) {
  +case ARMMMUIdx_S1SE0:
  +case ARMMMUIdx_S1NSE0:
 
 
  The get_phys_addr path filters out ARMMMUIdx_S12NSE0 so calls to this
  function in this context don't matter, but what if it is called outside
 this
  context?  How should it handle this index?

 g_assert_not_reached(),  but it didn't seem worth cluttering
 the switch with a bunch of extra labels just to assert that
 they weren't reachable.


​I see how it could clutter things, but given that the routine is generic
we probably should just like we do in regime_el().​



 thanks
 -- PMM



[Qemu-devel] [PATCH RESEND 30/50] blockdev: Check BB validity in change-backing-file

2015-01-27 Thread Max Reitz
Call blk_is_available() before using blk_bs() to obtain the root
BlockDriverState behind the BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4bd52b8..7f4470f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2821,7 +2821,8 @@ void qmp_change_backing_file(const char *device,
  const char *backing_file,
  Error **errp)
 {
-BlockDriverState *bs = NULL;
+BlockBackend *blk;
+BlockDriverState *bs;
 AioContext *aio_context;
 BlockDriverState *image_bs = NULL;
 Error *local_err = NULL;
@@ -2830,15 +2831,21 @@ void qmp_change_backing_file(const char *device,
 int ret;
 
 /* find the top layer BDS of the chain */
-bs = bdrv_find(device);
-if (!bs) {
+blk = blk_by_name(device);
+if (!blk) {
 error_set(errp, QERR_DEVICE_NOT_FOUND, device);
 return;
 }
 
-aio_context = bdrv_get_aio_context(bs);
+aio_context = blk_get_aio_context(blk);
 aio_context_acquire(aio_context);
 
+if (!blk_is_available(blk)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+goto out;
+}
+bs = blk_bs(blk);
+
 image_bs = bdrv_lookup_bs(NULL, image_node_name, local_err);
 if (local_err) {
 error_propagate(errp, local_err);
-- 
2.1.0




Re: [Qemu-devel] [PATCH 10/50] block: Remove wr_highest_offset from BlockAcctStats

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 BlockAcctStats contains statistics about the data transferred from and
 to the device; wr_highest_offset does not fit in with the rest.
 
 Furthermore, those statistics are supposed to be specific for a certain
 device and not necessarily for a BDS (see the comment above
 bdrv_get_stats()); on the other hand, wr_highest_offset may be a rather
 important information to know for each BDS. When BlockAcctStats is
 finally removed from the BDS, we will want to keep wr_highest_offset in
 the BDS.

Yes, I recently did work in libvirt to expose wr_highest_offset of
backing images during block commit (qemu still isn't populating it on
images opened only for read, but the point remains that it is a
statistic tied to the BDS, not the BB).

On the other hand, even the other statistics might make sense on both
BDS and BB level (at the BB level, how many bytes has the guest
read/written; at the BDS level, how many bytes were serviced by the
active layer vs. delegated to a backing layer).

I'm not sure if we are set up for that fine of a level of reporting yet,
but we shouldn't make it hard to implement later.  But for now, I agree
with separating the definite BDS-only stat, leaving the rest of the
struct usable for either BDS or BB.

 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block.c| 4 +++-
  block/accounting.c | 9 -
  block/qapi.c   | 4 ++--
  include/block/accounting.h | 3 ---
  include/block/block_int.h  | 3 +++
  5 files changed, 8 insertions(+), 15 deletions(-)
 
 diff --git a/block.c b/block.c

 +++ b/include/block/block_int.h
 @@ -366,6 +366,9 @@ struct BlockDriverState {
  /* I/O stats (display with info blockstats). */
  BlockAcctStats stats;
  
 +/* Highest sector index written to */
 +uint64_t wr_highest_sector;

Umm, now would be a great time to track this in bytes instead of
sectors, if that is not too difficult to do.

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH RESEND 33/50] blockdev: Respect NULL BDS in do_drive_del()

2015-01-27 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index f3091df..f82b20c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2019,7 +2019,6 @@ int do_drive_del(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 error_report(Device '%s' not found, id);
 return -1;
 }
-bs = blk_bs(blk);
 
 if (!blk_legacy_dinfo(blk)) {
 error_report(Deleting device added with blockdev-add
@@ -2027,10 +2026,11 @@ int do_drive_del(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 return -1;
 }
 
-aio_context = bdrv_get_aio_context(bs);
+aio_context = blk_get_aio_context(blk);
 aio_context_acquire(aio_context);
 
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, local_err)) {
+bs = blk_bs(blk);
+if (bs  bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, local_err)) {
 error_report(%s, error_get_pretty(local_err));
 error_free(local_err);
 aio_context_release(aio_context);
@@ -2039,8 +2039,10 @@ int do_drive_del(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 
 /* quiesce block driver; prevent further io */
 bdrv_drain_all();
-bdrv_flush(bs);
-bdrv_close(bs);
+if (bs) {
+bdrv_flush(bs);
+bdrv_close(bs);
+}
 
 /* if we have a device attached to this BlockDriverState
  * then we need to make the drive anonymous until the device
-- 
2.1.0




Re: [Qemu-devel] [PATCH 09/11] target-arm: Use mmu_idx in get_phys_addr()

2015-01-27 Thread Peter Maydell
On 27 January 2015 at 19:49, Greg Bellows greg.bell...@linaro.org wrote:
 On Tue, Jan 27, 2015 at 12:12 PM, Peter Maydell peter.mayd...@linaro.org
 wrote:
 g_assert_not_reached(),  but it didn't seem worth cluttering
 the switch with a bunch of extra labels just to assert that
 they weren't reachable.


 I see how it could clutter things, but given that the routine is generic we
 probably should just like we do in regime_el().

Not a big deal, so I'll add them, but the routine isn't generic --
it's purely a local utility routine for the benefit of the
get_phys_addr family of functions and not intended to be called
from elsewhere.

-- PMM



Re: [Qemu-devel] [PATCH V2 4/4] target-arm: Add missing SP_ELx register definition

2015-01-27 Thread Greg Bellows
On Tue, Jan 27, 2015 at 1:03 PM, Peter Maydell peter.mayd...@linaro.org
wrote:

 On 23 January 2015 at 16:17, Greg Bellows greg.bell...@linaro.org wrote:
  Added CP register definitions for SP_EL1 and SP_EL2.
 
  Signed-off-by: Greg Bellows greg.bell...@linaro.org
  Reviewed-by: Peter Maydell peter.mayd...@linaro.org
 
  ---
 
  v1 - v2
  - Remove unnecessary accessfn for SP_EL1/2
  - Revert SP_EL0 accessfn name to sp_el0_access
  ---
   target-arm/helper.c | 8 
   1 file changed, 8 insertions(+)
 
  diff --git a/target-arm/helper.c b/target-arm/helper.c
  index 29f3b62..79c54a9 100644
  --- a/target-arm/helper.c
  +++ b/target-arm/helper.c
  @@ -2329,6 +2329,10 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 .access = PL1_RW, .accessfn = sp_el0_access,
 .type = ARM_CP_NO_MIGRATE,
 .fieldoffset = offsetof(CPUARMState, sp_el[0]) },
  +{ .name = SP_EL1, .state = ARM_CP_STATE_AA64,
  +  .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 1, .opc2 = 0,
  +  .access = PL2_RW, .type = ARM_CP_NO_MIGRATE,
  +  .fieldoffset = offsetof(CPUARMState, sp_el[1]) },
   { .name = SPSel, .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 0, .crn = 4, .crm = 2, .opc2 = 0,
 .type = ARM_CP_NO_MIGRATE,
  @@ -2410,6 +2414,10 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
 .access = PL2_RW, .writefn = vbar_write,
 .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),
 .resetvalue = 0 },
  +{ .name = SP_EL2, .state = ARM_CP_STATE_AA64,
  +  .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
  +  .access = PL3_RW, .type = ARM_CP_NO_MIGRATE,
  +  .fieldoffset = offsetof(CPUARMState, sp_el[2]) },
   REGINFO_SENTINEL
   };

 As I was assembling my target-arm queue I found that this patch
 and the 'split ARM_CP_NO_MIGRATE' patch semantically conflict;
 since this patch happened to be earlier in the queue than that
 one, I've resolved this by adding changes to the 'split' patch
 which change these ARM_CP_NO_MIGRATE uses to ARM_CP_ALIAS, in
 line with how we handled the SP_EL0 regdef.

 thanks
 -- PMM


​That makes sense.​


Re: [Qemu-devel] [PATCH 0/6] linux-user: Fix various clang warnings

2015-01-27 Thread Peter Maydell
Ping^2 ?

thanks
-- PMM

On 21 January 2015 at 10:23, Riku Voipio riku.voi...@iki.fi wrote:
 Hi,

 On Tue, Jan 20, 2015 at 02:54:20PM +, Peter Maydell wrote:
 Ping!

 I've just updated my linux-user-for-upstream tree. I'll get it
 reviewed/tested tonight.

 thanks
 -- PMM

 On 8 January 2015 at 12:19, Peter Maydell peter.mayd...@linaro.org wrote:
  This patchset fixes warnings produced by clang in the linux-user code.
  Mostly this is deleting or marking unused functions/data, but it
  does include a genuine bugfix for Alpha.
 
  I think that this means I have patches out on the list now for
  all the clang warnings we currently generate; maybe some day soon
  we can enable warnings-as-errors...
 
  Peter Maydell (6):
linux-user/signal.c: Remove current_exec_domain_sig()
linux-user/alpha: Add define for NR_shmat to enable shmat syscall
linux-user/arm/nwfpe: Delete unused aCC array
linux-user/main.c: Call cpu_exec_start/end on all target archs
linux-user/main.c: Mark end_exclusive() as possibly unused
linux-user/signal.c: Remove unnecessary wrapper copy_siginfo_to_user
 
   linux-user/alpha/syscall_nr.h   |  4 +++
   linux-user/arm/nwfpe/fpopcode.c | 22 ---
   linux-user/main.c   | 20 +-
   linux-user/signal.c | 59 
  -
   4 files changed, 40 insertions(+), 65 deletions(-)



Re: [Qemu-devel] [PATCH 05/50] block: Fix BB AIOCB AioContext without BDS

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 Fix the BlockBackend's AIOCB AioContext for aborting AIO in case there
 is no BDS. If there is no implementation of AIOCBInfo::get_aio_context()
 the AioContext is derived from the BDS the AIOCB belongs to. If that BDS
 is NULL (because it has been removed from the BB) this will not work.
 
 This patch makes blk_get_aio_context() fall back to the main loop
 context if the BDS pointer is NULL and implements
 AIOCBInfo::get_aio_context() (blk_aiocb_get_aio_context()) which invokes
 blk_get_aio_context().
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block/block-backend.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)
 

Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH RESEND 00/50] blockdev: BlockBackend and media

2015-01-27 Thread Max Reitz
** This is a resend due to me messing up John's CC in the first thread.
Sorry for the superfluous mails, but I feel like it's better to bother
you all with the same mails again than to keep bothering innocent
people. **

This series reworks a lot regarding BlockBackend and media. It is
essentially a v3 to the blockdev: Add blockdev-change-medium with
read-only option series (which is in fact a part of this series), but
of course does a lot more.

Basically, this series allows empty BlockBackends, that is BBs without a
BDS tree.

Before this series, empty drives are represented by a BlockBackend with
an empty BDS attached to it (a BDS with a NULL driver). However, now we
have BlockBackends, thus an empty drive should be represented by a
BlockBackend without any BDS tree attached to it. This is what this
series does.


I am CC'ing a lot of people (basically everyone who is working on the
block layer) due to the fact that this series touches a lot of places in
the block layer. Please pick out patches that seem to be relevant to
your area of expertise (e.g. there is an FDC tray patch for John; there
are some block job patches for Jeff; etc. pp.). I want to explicitly
encourage you to review only parts of this series! (Of course, reviewing
everything is fine, too :-))

Also, brace for a follow-up regarding bdrv_close_all() (there is a nice
bug in there which I found while working on this series).


This series depends on v3 (or any later version) of my series
'block: Remove growable, add blk_new_open()'.


- Patches 1 and 2 make it possible to use blockdev-add without creating
  a BlockBackend. This is necessary because the QMP command introduced
  in patch 42 (blockdev-insert-medium) will insert a BDS tree (a medium)
  into a BlockBackend (a drive). Creating a BlockBackend for such a BDS
  tree would both be a hassle and a waste, so this makes it possible to
  omit creating a BB.
  Patches 35 and 36 are kind of a follow-up to these; but patch 35
  depends on patch 34 which is the reason why there is a large gap
  between patch 2 and 35.

- Patch 3 implements a tray status for floppy disk drives. See the
  commit message for more information on what this means.

- Patches 3 to 33 basically all prepare the block layer for having BBs
  without BDS trees. I will only list the most notable ones (patch 3,
  for instance, is not notable).
  They do so by moving some information into the BlockBackend, and
  primarily by intercepting accesses to empty BBs in the BB layer and
  resorting to default actions there.
  Furthermore, hopefully all non-BB calls to bdrv_*() functions are
  guarded by checking whether that BDS actually exists.

- Patch 6 makes blk_is_inserted() return true only if there is a BDS
  tree; furthermore, blk_is_available() is added which additionally
  checks whether the guest device tray is closed.

- Patches 7 and 8 are some kind of follow-up to patch 6; they make
  bdrv_is_inserted() actually useful (unless I'm not mistaken; maybe I
  am and they make bdrv_is_inserted() actually useless).

- Patches 9, 11 and 12 move some (root!) BDS fields into the BB, because
  that is where they belong. This way they survive a medium (BDS tree)
  change.

- Patch 10 is necessary because the structure moved in patch 11
  (BlockAcctStats) contains one field which does belong into the BDS
  (wr_highest_offset). Thus, this field is moved out of that structure
  into the BDS here.

- Patch 13 adds a structure to the BB which can save some options from
  the root BDS so they survive a medium change. In theory, those options
  should probably not survive a medium change (they are not really
  drive- but medium-related), but this is how the 'change' command
  always handled it so this structure (the BlockBackendRootState, BBRS)
  is required for compatibility.
  One of these options is the read-only status, for example.

- Patches 17 to 30 and patches 32 and 33 prepare functions in the block
  layer which directly access a BDS to cope with a non-existing BDS
  tree. Patch 31 is a prerequisite for patch 32.

- Patch 34 gets down to business: Empty drives no longer have a BDS
  tree.

- Patches 35 and 36 are, as described above, a follow-up to patch 1.

- Patch 37 is the counterpart to patch 31, obviously.

- Patches 38 to 41 implement the basic QMP operations for tray and
  medium operation: blockdev-open-tray, blockdev-close-tray,
  blockdev-remove-medium and blockdev-insert-medium.

- Patches 42 and 43 reimplement 'eject' and 'change' using these new
  medium operations.

- With me now knowing exactly what 'change' does (because I
  reimplemented it), patch 44 became possible.

- Patches 45 to 48 are from v2 of series (slightly modified)
  blockdev: Add blockdev-change-medium with read-only option
  One modification are notes in patch 45 that blockdev-change-medium is
  preferred over simply 'change' [Eric] and what atomic operations
  blockdev-change-medium itself performs.

- Patch 50 adds a test for 'change' and 

[Qemu-devel] [PATCH RESEND 02/50] iotests: Only create BB if necessary

2015-01-27 Thread Max Reitz
Tests 071 and 081 test giving references in blockdev-add. It is not
necessary to create a BlockBackend here, so omit it.

Signed-off-by: Max Reitz mre...@redhat.com
---
 tests/qemu-iotests/071 | 50 ++
 tests/qemu-iotests/071.out | 12 +++
 tests/qemu-iotests/081 | 14 -
 tests/qemu-iotests/081.out |  5 +++--
 4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index 9eaa49b..68bedd4 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -104,11 +104,20 @@ echo
 echo === Testing blkdebug on existing block device ===
 echo
 
-run_qemu -drive file=$TEST_IMG,format=raw,if=none,id=drive0 EOF
+run_qemu EOF
 { execute: qmp_capabilities }
 { execute: blockdev-add,
 arguments: {
 options: {
+node-name: drive0,
+driver: file,
+filename: $TEST_IMG
+}
+}
+}
+{ execute: blockdev-add,
+arguments: {
+options: {
 driver: $IMGFMT,
 id: drive0-debug,
 file: {
@@ -133,11 +142,23 @@ echo
 echo === Testing blkverify on existing block device ===
 echo
 
-run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=drive0 EOF
+run_qemu EOF
 { execute: qmp_capabilities }
 { execute: blockdev-add,
 arguments: {
 options: {
+node-name: drive0,
+driver: $IMGFMT,
+file: {
+driver: file,
+filename: $TEST_IMG
+}
+}
+}
+}
+{ execute: blockdev-add,
+arguments: {
+options: {
 driver: blkverify,
 id: drive0-verify,
 test: drive0,
@@ -163,11 +184,23 @@ echo
 echo === Testing blkverify on existing raw block device ===
 echo
 
-run_qemu -drive file=$TEST_IMG.base,format=raw,if=none,id=drive0 EOF
+run_qemu EOF
 { execute: qmp_capabilities }
 { execute: blockdev-add,
 arguments: {
 options: {
+node-name: drive0,
+driver: raw,
+file: {
+driver: file,
+filename: $TEST_IMG.base
+}
+}
+}
+}
+{ execute: blockdev-add,
+arguments: {
+options: {
 driver: blkverify,
 id: drive0-verify,
 test: {
@@ -193,11 +226,20 @@ echo
 echo === Testing blkdebug's set-state through QMP ===
 echo
 
-run_qemu -drive file=$TEST_IMG,format=raw,if=none,id=drive0 EOF
+run_qemu EOF
 { execute: qmp_capabilities }
 { execute: blockdev-add,
 arguments: {
 options: {
+node-name: drive0,
+driver: file,
+filename: $TEST_IMG
+}
+}
+}
+{ execute: blockdev-add,
+arguments: {
+options: {
 driver: $IMGFMT,
 id: drive0-debug,
 file: {
diff --git a/tests/qemu-iotests/071.out b/tests/qemu-iotests/071.out
index 9205ce2..c8ecfaf 100644
--- a/tests/qemu-iotests/071.out
+++ b/tests/qemu-iotests/071.out
@@ -42,10 +42,11 @@ read failed: Input/output error
 
 === Testing blkdebug on existing block device ===
 
-Testing: -drive file=TEST_DIR/t.IMGFMT,format=raw,if=none,id=drive0
+Testing:
 QMP_VERSION
 {return: {}}
 {return: {}}
+{return: {}}
 read failed: Input/output error
 {return: }
 {return: {}}
@@ -58,28 +59,31 @@ QEMU_PROG: Failed to flush the refcount block cache: 
Input/output error
 
 === Testing blkverify on existing block device ===
 
-Testing: -drive file=TEST_DIR/t.IMGFMT,format=IMGFMT,if=none,id=drive0
+Testing:
 QMP_VERSION
 {return: {}}
 {return: {}}
+{return: {}}
 blkverify: read sector_num=0 nb_sectors=1 contents mismatch in sector 0
 
 
 === Testing blkverify on existing raw block device ===
 
-Testing: -drive file=TEST_DIR/t.IMGFMT.base,format=raw,if=none,id=drive0
+Testing:
 QMP_VERSION
 {return: {}}
 {return: {}}
+{return: {}}
 blkverify: read sector_num=0 nb_sectors=1 contents mismatch in sector 0
 
 
 === Testing blkdebug's set-state through QMP ===
 
-Testing: -drive file=TEST_DIR/t.IMGFMT,format=raw,if=none,id=drive0
+Testing:
 QMP_VERSION
 {return: {}}
 {return: {}}
+{return: {}}
 read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 {return: }
diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
index d9b042c..5c8a8fa 100755
--- a/tests/qemu-iotests/081
+++ b/tests/qemu-iotests/081
@@ -101,11 +101,23 @@ $QEMU_IO -c open -o $quorum -c read -P 0x32 0 $size | 
_filter_qemu_io
 echo
 echo == checking mixed reference/option specification ==
 
-run_qemu -drive file=$TEST_DIR/2.raw,format=$IMGFMT,if=none,id=drive2 EOF
+run_qemu EOF
 { execute: qmp_capabilities }
 { execute: blockdev-add,
 arguments: {
 options: {
+node-name: drive2,
+driver: raw,
+file: {
+driver: file,
+filename: $TEST_DIR/2.raw
+}
+}
+}
+}
+{ execute: blockdev-add,
+arguments: {
+options: {
 

[Qemu-devel] [PATCH RESEND 29/50] blockdev: Check BB validity in find_block_job()

2015-01-27 Thread Max Reitz
Call blk_is_available() before using blk_bs() to obtain the root
BlockDriverState behind the BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4e12061..4bd52b8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2698,25 +2698,35 @@ out:
 /* Get the block job for a given device name and acquire its AioContext */
 static BlockJob *find_block_job(const char *device, AioContext **aio_context)
 {
+BlockBackend *blk;
 BlockDriverState *bs;
 
-bs = bdrv_find(device);
-if (!bs) {
+*aio_context = NULL;
+
+blk = blk_by_name(device);
+if (!blk) {
 goto notfound;
 }
 
-*aio_context = bdrv_get_aio_context(bs);
+*aio_context = blk_get_aio_context(blk);
 aio_context_acquire(*aio_context);
 
+if (!blk_is_available(blk)) {
+goto notfound;
+}
+bs = blk_bs(blk);
+
 if (!bs-job) {
-aio_context_release(*aio_context);
 goto notfound;
 }
 
 return bs-job;
 
 notfound:
-*aio_context = NULL;
+if (*aio_context) {
+aio_context_release(*aio_context);
+*aio_context = NULL;
+}
 return NULL;
 }
 
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 40/50] blockdev: Add blockdev-remove-medium

2015-01-27 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c   | 25 +
 qapi/block-core.json | 13 +
 qmp-commands.hx  | 43 +++
 3 files changed, 81 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index d6a3fdf..17785b8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2063,6 +2063,31 @@ void qmp_blockdev_close_tray(const char *device, Error 
**errp)
 blk_dev_change_media_cb(blk, true);
 }
 
+void qmp_blockdev_remove_medium(const char *device, Error **errp)
+{
+BlockBackend *blk;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+if (!blk_dev_has_removable_media(blk)) {
+error_setg(errp, Device '%s' is not removable, device);
+return;
+}
+
+if (!blk_dev_is_tray_open(blk)) {
+error_setg(errp, Tray of device '%s' is not open, device);
+return;
+}
+
+if (blk_bs(blk)) {
+blk_remove_bs(blk);
+}
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 802734a..1ace69d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1753,6 +1753,19 @@
 { 'command': 'blockdev-close-tray',
   'data': { 'device': 'str' } }
 
+##
+# @blockdev-remove-medium:
+#
+# Removes a medium (a block driver state tree) from a block device. That block
+# device's tray must currently be open.
+#
+# @device: block device name
+#
+# Since: 2.3
+##
+{ 'command': 'blockdev-remove-medium',
+  'data': { 'device': 'str' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5927507..3d5c10c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3711,6 +3711,49 @@ Example (1):
 EQMP
 
 {
+.name   = blockdev-remove-medium,
+.args_type  = device:s,
+.mhandler.cmd_new = qmp_marshal_input_blockdev_remove_medium,
+},
+
+SQMP
+blockdev-remove-medium
+--
+
+Removes a medium (a block driver state tree) from a block device. That block
+device's tray must currently be open.
+
+Arguments:
+
+- device: block device name (json-string)
+
+Example (1):
+
+- { execute: blockdev-remove-medium,
+ arguments: { device: ide1-cd0 } }
+
+- { error: { class: GenericError,
+desc: Tray of device 'ide1-cd0' is not open } }
+
+- { execute: blockdev-open-tray,
+ arguments: { device: ide1-cd0 } }
+
+- { timestamp: { seconds: 1418751627,
+microseconds: 549958 },
+ event: DEVICE_TRAY_MOVED,
+ data: { device: ide1-cd0,
+   tray-open: true } }
+
+- { return: {} }
+
+- { execute: blockdev-remove-medium,
+ arguments: { device: ide1-cd0 } }
+
+- { return: {} }
+
+EQMP
+
+{
 .name   = query-named-block-nodes,
 .args_type  = ,
 .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes,
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 32/50] blockdev: Check BB validity in eject and change

2015-01-27 Thread Max Reitz
Both commands will be reimplemented in a follow-up to this patch so this
does not need to be nice, it just has to work.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 7f4470f..f3091df 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1756,10 +1756,10 @@ static void eject_device(BlockBackend *blk, int force, 
Error **errp)
 BlockDriverState *bs = blk_bs(blk);
 AioContext *aio_context;
 
-aio_context = bdrv_get_aio_context(bs);
+aio_context = blk_get_aio_context(blk);
 aio_context_acquire(aio_context);
 
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+if (bs  bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
 goto out;
 }
 if (!blk_dev_has_removable_media(blk)) {
@@ -1777,7 +1777,9 @@ static void eject_device(BlockBackend *blk, int force, 
Error **errp)
 }
 }
 
-bdrv_close(bs);
+if (bs) {
+bdrv_close(bs);
+}
 
 out:
 aio_context_release(aio_context);
@@ -1830,18 +1832,21 @@ out:
 }
 
 /* Assumes AioContext is held */
-static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
+static void qmp_bdrv_open_encrypted(BlockDriverState **pbs,
+const char *filename,
 int bdrv_flags, BlockDriver *drv,
 const char *password, Error **errp)
 {
+BlockDriverState *bs;
 Error *local_err = NULL;
 int ret;
 
-ret = bdrv_open(bs, filename, NULL, NULL, bdrv_flags, drv, local_err);
+ret = bdrv_open(pbs, filename, NULL, NULL, bdrv_flags, drv, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
 return;
 }
+bs = *pbs;
 
 if (bdrv_key_required(bs)) {
 if (password) {
@@ -1865,6 +1870,7 @@ void qmp_change_blockdev(const char *device, const char 
*filename,
 AioContext *aio_context;
 BlockDriver *drv = NULL;
 int bdrv_flags;
+bool new_bs;
 Error *err = NULL;
 
 blk = blk_by_name(device);
@@ -1873,12 +1879,13 @@ void qmp_change_blockdev(const char *device, const char 
*filename,
 return;
 }
 bs = blk_bs(blk);
+new_bs = !bs;
 
-aio_context = bdrv_get_aio_context(bs);
+aio_context = blk_get_aio_context(blk);
 aio_context_acquire(aio_context);
 
 if (format) {
-drv = bdrv_find_whitelisted_format(format, bs-read_only);
+drv = bdrv_find_whitelisted_format(format, blk_is_read_only(blk));
 if (!drv) {
 error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
 goto out;
@@ -1891,10 +1898,18 @@ void qmp_change_blockdev(const char *device, const char 
*filename,
 goto out;
 }
 
-bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
-bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
+bdrv_flags = blk_is_read_only(blk) ? 0 : BDRV_O_RDWR;
+bdrv_flags |= blk_get_root_state(blk)-open_flags  ~BDRV_O_RDWR;
 
-qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
+qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, err);
+if (err) {
+error_propagate(errp, err);
+} else if (new_bs) {
+blk_insert_bs(blk, bs);
+/* Has been sent automatically by bdrv_open() if blk_bs(blk) was not
+ * NULL */
+blk_dev_change_media_cb(blk, true);
+}
 
 out:
 aio_context_release(aio_context);
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 46/50] hmp: Use blockdev-change-medium for change command

2015-01-27 Thread Max Reitz
Use separate code paths for the two overloaded functions of the 'change'
HMP command, and invoke the 'blockdev-change-medium' QMP command if used
on a block device (by calling qmp_blockdev_change_medium()).

Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 hmp.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/hmp.c b/hmp.c
index f806387..300e7d8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1180,22 +1180,25 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 const char *arg = qdict_get_try_str(qdict, arg);
 Error *err = NULL;
 
-if (strcmp(device, vnc) == 0 
-(strcmp(target, passwd) == 0 ||
- strcmp(target, password) == 0)) {
-if (!arg) {
-monitor_read_password(mon, hmp_change_read_arg, NULL);
+if (strcmp(device, vnc) == 0) {
+if (strcmp(target, passwd) == 0 ||
+strcmp(target, password) == 0) {
+if (!arg) {
+monitor_read_password(mon, hmp_change_read_arg, NULL);
+return;
+}
+}
+qmp_change(vnc, target, !!arg, arg, err);
+} else {
+qmp_blockdev_change_medium(device, target, !!arg, arg, err);
+if (err 
+error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
+error_free(err);
+monitor_read_block_device_key(mon, device, NULL, NULL);
 return;
 }
 }
 
-qmp_change(device, target, !!arg, arg, err);
-if (err 
-error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
-error_free(err);
-monitor_read_block_device_key(mon, device, NULL, NULL);
-return;
-}
 hmp_handle_error(mon, err);
 }
 
-- 
2.1.0




Re: [Qemu-devel] [PATCH 1/2] vl.c: Fix error messages when parsing maxmem parameters

2015-01-27 Thread Eric Blake
On 01/26/2015 08:31 AM, Peter Krempa wrote:
 Produce more human readable error messages and fix few spelling
 mistakes.
 
 Also remove a redundant check for the max memory size.
 
 Signed-off-by: Peter Krempa pkre...@redhat.com
 ---
  vl.c | 22 +++---
  1 file changed, 7 insertions(+), 15 deletions(-)
 
 diff --git a/vl.c b/vl.c
 index 983259b..cdc920c 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -2694,29 +2694,21 @@ static void set_memory_options(uint64_t *ram_slots, 
 ram_addr_t *maxram_size)
  uint64_t slots;
 
  sz = qemu_opt_get_size(opts, maxmem, 0);
 -if (sz  ram_size) {
 -error_report(invalid -m option value: maxmem 
 -(0x% PRIx64 ) = initial memory (0x
 -RAM_ADDR_FMT ), sz, ram_size);
 +if (sz = ram_size) {

Why are we changing from '' to '='?  I think the error was in the
message, not in the code, and that setting max == size should be
allowed. [1]

 +error_report(invalid value of -m option maxmem: 
 + maximum memory size (0x% PRIx64 ) must be 
 greater 
 + than initial memory size (0x  RAM_ADDR_FMT ),

Why two spaces?  If I'm correct that we want '' in the condition, then
the wording 'must be at least the initial memory size' would be better.

 + sz, ram_size);
  exit(EXIT_FAILURE);
  }
 
  slots = qemu_opt_get_number(opts, slots, 0);
  if ((sz  ram_size)  !slots) {
 -error_report(invalid -m option value: maxmem 
 -(0x% PRIx64 ) more than initial memory (0x
 -RAM_ADDR_FMT ) but no hotplug slots where 
 -specified, sz, ram_size);
 +error_report(invalid value of -m option: maxmem was specified, 
 + but no hotplug slots were specified);
  exit(EXIT_FAILURE);
  }
 
 -if ((sz = ram_size)  slots) {
 -error_report(invalid -m option value:  %
 -PRIu64  hotplug slots where specified but 
 -maxmem (0x% PRIx64 ) = initial memory (0x
 -RAM_ADDR_FMT ), slots, sz, ram_size);
 -exit(EXIT_FAILURE);
 -}

Okay, I see.  This is dead if condition [1] is changed to '=', but
still reachable (sz == ram_size) if condition [1] is left at ''.  Maybe
better logic would be:

if (sz  ram_size) {
max cannot be less than memory
}
if (sz  ram_size) {
if (!slots) {
max cannot be larger than size without hotplug slots
}
} else if (slots) {
max must be larger than size to support hotplug slots
}

to allow max==ram_size when slots is not present.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 01/50] blockdev: Allow creation of BDS trees without BB

2015-01-27 Thread Max Reitz

On 2015-01-27 at 13:22, Eric Blake wrote:

On 01/26/2015 09:02 AM, Max Reitz wrote:

If the id field is missing from the options given to blockdev-add,
just omit the BlockBackend and create the BlockDriverState tree alone.

Signed-off-by: Max Reitz mre...@redhat.com
---
  blockdev.c | 38 +-
  tests/qemu-iotests/087 | 20 
  tests/qemu-iotests/087.out | 12 
  3 files changed, 25 insertions(+), 45 deletions(-)

I'd feel more comfortable if this patch also touched
qapi/block-core.json to mention the change in policy on
BlockdevOptionsBase.


Will do.


Also, should 'node-name' be mandatory when
omitting 'id'?


Seems good to me. Not specifying a node-name doesn't make a whole lot of 
sense, except for when you really badly want qemu to create useless 
structures in memory.


Max



Re: [Qemu-devel] [PATCH v1 2/2] target_arm: Parameterise the irq lines for armv7m_init

2015-01-27 Thread Peter Maydell
On 24 January 2015 at 08:02, Alistair Francis alistai...@gmail.com wrote:
 This patch allows the board to specifiy the number of NVIC interrupt
 lines when using armv7m_init.

 Signed-off-by: Alistair Francis alistai...@gmail.com
 Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 ---
  /* FIXME: make this local state.  */
 -static qemu_irq pic[64];
 +qemu_irq *pic = g_new(qemu_irq, num_irq);

The FIXME is now stale and can be removed.

I've made that minor change and applied this to target-arm.next.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 06/50] block: Add blk_is_available()

2015-01-27 Thread Max Reitz

On 2015-01-27 at 14:15, Eric Blake wrote:

On 01/26/2015 09:02 AM, Max Reitz wrote:

blk_is_available() returns true iff the BDS is inserted (which means
blk_bs() is not NULL and bdrv_is_inserted() returns true) and if the
tray of the guest device is closed.

blk_is_inserted() is changed to return true only if blk_bs() is not
NULL.

Signed-off-by: Max Reitz mre...@redhat.com
---
  block/block-backend.c  | 7 ++-
  include/sysemu/block-backend.h | 1 +
  2 files changed, 7 insertions(+), 1 deletion(-)

+++ b/include/sysemu/block-backend.h
@@ -127,6 +127,7 @@ int blk_enable_write_cache(BlockBackend *blk);
  void blk_set_enable_write_cache(BlockBackend *blk, bool wce);
  void blk_invalidate_cache(BlockBackend *blk, Error **errp);
  int blk_is_inserted(BlockBackend *blk);

Should we change blk_is_inserted to return bool?


Seems like a good opportunity to. Will do.


+bool blk_is_available(BlockBackend *blk);

Looks reasonable.
Reviewed-by: Eric Blake ebl...@redhat.com


Thank you for tackling this series!

Max



Re: [Qemu-devel] [PATCH 08/50] block/quorum: Implement bdrv_is_inserted()

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 bdrv_is_inserted() should be invoked recursively on the children of
 quorum.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block/quorum.c | 16 
  1 file changed, 16 insertions(+)
 
 diff --git a/block/quorum.c b/block/quorum.c
 index 437b122..7811c4a 100644
 --- a/block/quorum.c
 +++ b/block/quorum.c
 @@ -1064,6 +1064,20 @@ static void quorum_refresh_filename(BlockDriverState 
 *bs)
  bs-full_open_options = opts;
  }
  
 +static int quorum_is_inserted(BlockDriverState *bs)
 +{
 +BDRVQuorumState *s = bs-opaque;
 +int i;
 +
 +for (i = 0; i  s-num_children; i++) {
 +if (!bdrv_is_inserted(s-bs[i])) {
 +return 0;

If you convert the callback to return bool, this may need minor tweaks.
 But they don't affect correctness;

Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH RESEND 15/50] block: Fail requests to empty BlockBackend

2015-01-27 Thread Max Reitz
If there is no BlockDriverState in a BlockBackend or if the tray of the
guest device is open, fail all requests (where that is possible) with
-ENOMEDIUM.

The reason the status of the guest device is taken into account is
because once the guest device's tray is opened, any request on the same
BlockBackend as the guest uses should fail. If the BDS tree is supposed
to be usable even after ejecting it from the guest, a different
BlockBackend must be used.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/block-backend.c | 43 ++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 62be370..4f3122a 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -507,7 +507,7 @@ static int blk_check_byte_request(BlockBackend *blk, 
int64_t offset,
 return -EIO;
 }
 
-if (!blk_is_inserted(blk)) {
+if (!blk_is_available(blk)) {
 return -ENOMEDIUM;
 }
 
@@ -635,6 +635,10 @@ int blk_pwrite(BlockBackend *blk, int64_t offset, const 
void *buf, int count)
 
 int64_t blk_getlength(BlockBackend *blk)
 {
+if (!blk_is_available(blk)) {
+return -ENOMEDIUM;
+}
+
 return bdrv_getlength(blk-bs);
 }
 
@@ -670,6 +674,10 @@ BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t 
sector_num,
 BlockAIOCB *blk_aio_flush(BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque)
 {
+if (!blk_is_available(blk)) {
+return abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
+}
+
 return bdrv_aio_flush(blk-bs, cb, opaque);
 }
 
@@ -711,12 +719,20 @@ int blk_aio_multiwrite(BlockBackend *blk, BlockRequest 
*reqs, int num_reqs)
 
 int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
 {
+if (!blk_is_available(blk)) {
+return -ENOMEDIUM;
+}
+
 return bdrv_ioctl(blk-bs, req, buf);
 }
 
 BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
   BlockCompletionFunc *cb, void *opaque)
 {
+if (!blk_is_available(blk)) {
+return abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
+}
+
 return bdrv_aio_ioctl(blk-bs, req, buf, cb, opaque);
 }
 
@@ -732,11 +748,19 @@ int blk_co_discard(BlockBackend *blk, int64_t sector_num, 
int nb_sectors)
 
 int blk_co_flush(BlockBackend *blk)
 {
+if (!blk_is_available(blk)) {
+return -ENOMEDIUM;
+}
+
 return bdrv_co_flush(blk-bs);
 }
 
 int blk_flush(BlockBackend *blk)
 {
+if (!blk_is_available(blk)) {
+return -ENOMEDIUM;
+}
+
 return bdrv_flush(blk-bs);
 }
 
@@ -853,6 +877,11 @@ void blk_set_enable_write_cache(BlockBackend *blk, bool 
wce)
 
 void blk_invalidate_cache(BlockBackend *blk, Error **errp)
 {
+if (!blk-bs) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, blk-name);
+return;
+}
+
 bdrv_invalidate_cache(blk-bs, errp);
 }
 
@@ -1003,6 +1032,10 @@ int blk_write_compressed(BlockBackend *blk, int64_t 
sector_num,
 
 int blk_truncate(BlockBackend *blk, int64_t offset)
 {
+if (!blk_is_available(blk)) {
+return -ENOMEDIUM;
+}
+
 return bdrv_truncate(blk-bs, offset);
 }
 
@@ -1019,11 +1052,19 @@ int blk_discard(BlockBackend *blk, int64_t sector_num, 
int nb_sectors)
 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
  int64_t pos, int size)
 {
+if (!blk_is_available(blk)) {
+return -ENOMEDIUM;
+}
+
 return bdrv_save_vmstate(blk-bs, buf, pos, size);
 }
 
 int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size)
 {
+if (!blk_is_available(blk)) {
+return -ENOMEDIUM;
+}
+
 return bdrv_load_vmstate(blk-bs, buf, pos, size);
 }
 
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 28/50] blockdev: Check BB validity in drive-mirror

2015-01-27 Thread Max Reitz
Call blk_is_available() before using blk_bs() to obtain the root
BlockDriverState behind the BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e335d06..4e12061 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2516,6 +2516,7 @@ void qmp_drive_mirror(const char *device, const char 
*target,
   bool has_on_target_error, BlockdevOnError 
on_target_error,
   Error **errp)
 {
+BlockBackend *blk;
 BlockDriverState *bs;
 BlockDriverState *source, *target_bs;
 AioContext *aio_context;
@@ -2555,19 +2556,20 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 return;
 }
 
-bs = bdrv_find(device);
-if (!bs) {
+blk = blk_by_name(device);
+if (!blk) {
 error_set(errp, QERR_DEVICE_NOT_FOUND, device);
 return;
 }
 
-aio_context = bdrv_get_aio_context(bs);
+aio_context = blk_get_aio_context(blk);
 aio_context_acquire(aio_context);
 
-if (!bdrv_is_inserted(bs)) {
+if (!blk_is_available(blk)) {
 error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
 goto out;
 }
+bs = blk_bs(blk);
 
 if (!has_format) {
 format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs-drv-format_name;
-- 
2.1.0




Re: [Qemu-devel] [PATCH 00/50] blockdev: BlockBackend and media

2015-01-27 Thread Max Reitz
Please do not reply to this thread any more; refer to the RESEND 
thread instead (I messed up John's CC).


If you have to reply to this thread, please replace j...@redhat.com by 
js...@redhat.com.


Sorry for the inconvience, everyone.

Max



[Qemu-devel] [PATCH RESEND 26/50] blockdev: Check BB validity in drive-backup

2015-01-27 Thread Max Reitz
Call blk_is_available() before using blk_bs() to obtain the root
BlockDriverState behind the BlockBackend (instead of calling
bdrv_is_inserted() after bdrv_find()).

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 431afcd..9476c72 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2323,6 +2323,7 @@ void qmp_drive_backup(const char *device, const char 
*target,
   bool has_on_target_error, BlockdevOnError 
on_target_error,
   Error **errp)
 {
+BlockBackend *blk;
 BlockDriverState *bs;
 BlockDriverState *target_bs;
 BlockDriverState *source = NULL;
@@ -2346,21 +2347,22 @@ void qmp_drive_backup(const char *device, const char 
*target,
 mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
 }
 
-bs = bdrv_find(device);
-if (!bs) {
+blk = blk_by_name(device);
+if (!blk) {
 error_set(errp, QERR_DEVICE_NOT_FOUND, device);
 return;
 }
 
-aio_context = bdrv_get_aio_context(bs);
+aio_context = blk_get_aio_context(blk);
 aio_context_acquire(aio_context);
 
 /* Although backup_run has this check too, we need to use bs-drv below, so
  * do an early check redundantly. */
-if (!bdrv_is_inserted(bs)) {
+if (!blk_is_available(blk)) {
 error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
 goto out;
 }
+bs = blk_bs(blk);
 
 if (!has_format) {
 format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs-drv-format_name;
-- 
2.1.0




Re: [Qemu-devel] [PATCH 10/50] block: Remove wr_highest_offset from BlockAcctStats

2015-01-27 Thread Max Reitz

On 2015-01-27 at 15:01, Eric Blake wrote:

On 01/26/2015 09:02 AM, Max Reitz wrote:

BlockAcctStats contains statistics about the data transferred from and
to the device; wr_highest_offset does not fit in with the rest.

Furthermore, those statistics are supposed to be specific for a certain
device and not necessarily for a BDS (see the comment above
bdrv_get_stats()); on the other hand, wr_highest_offset may be a rather
important information to know for each BDS. When BlockAcctStats is
finally removed from the BDS, we will want to keep wr_highest_offset in
the BDS.

Yes, I recently did work in libvirt to expose wr_highest_offset of
backing images during block commit (qemu still isn't populating it on
images opened only for read, but the point remains that it is a
statistic tied to the BDS, not the BB).

On the other hand, even the other statistics might make sense on both
BDS and BB level (at the BB level, how many bytes has the guest
read/written; at the BDS level, how many bytes were serviced by the
active layer vs. delegated to a backing layer).

I'm not sure if we are set up for that fine of a level of reporting yet,
but we shouldn't make it hard to implement later.  But for now, I agree
with separating the definite BDS-only stat, leaving the rest of the
struct usable for either BDS or BB.


Signed-off-by: Max Reitz mre...@redhat.com
---
  block.c| 4 +++-
  block/accounting.c | 9 -
  block/qapi.c   | 4 ++--
  include/block/accounting.h | 3 ---
  include/block/block_int.h  | 3 +++
  5 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
+++ b/include/block/block_int.h
@@ -366,6 +366,9 @@ struct BlockDriverState {
  /* I/O stats (display with info blockstats). */
  BlockAcctStats stats;
  
+/* Highest sector index written to */

+uint64_t wr_highest_sector;

Umm, now would be a great time to track this in bytes instead of
sectors, if that is not too difficult to do.


Fine with me, will do.

(on a different note, please refer to the RESEND thread or replace 
j...@redhat.com by js...@redhat.com in the CC list (my bad))


Max



[Qemu-devel] [PATCH RESEND 39/50] blockdev: Add blockdev-close-tray

2015-01-27 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c   | 22 ++
 qapi/block-core.json | 14 ++
 qmp-commands.hx  | 33 +
 3 files changed, 69 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index a0b9f17..d6a3fdf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2041,6 +2041,28 @@ out:
 }
 }
 
+void qmp_blockdev_close_tray(const char *device, Error **errp)
+{
+BlockBackend *blk;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+if (!blk_dev_has_removable_media(blk)) {
+error_setg(errp, Device '%s' is not removable, device);
+return;
+}
+
+if (!blk_dev_is_tray_open(blk)) {
+return;
+}
+
+blk_dev_change_media_cb(blk, true);
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 029f08b..802734a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1739,6 +1739,20 @@
   'data': { 'device': 'str',
 '*force': 'bool' } }
 
+##
+# @blockdev-close-tray:
+#
+# Closes a block device's tray. If there is a block driver state tree 
associated
+# with the block device (which is currently ejected), that tree will be loaded
+# as the medium.
+#
+# @device: block device name
+#
+# Since: 2.3
+##
+{ 'command': 'blockdev-close-tray',
+  'data': { 'device': 'str' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cfa1b98..5927507 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3678,6 +3678,39 @@ Example (1):
 EQMP
 
 {
+.name   = blockdev-close-tray,
+.args_type  = device:s,
+.mhandler.cmd_new = qmp_marshal_input_blockdev_close_tray,
+},
+
+SQMP
+blockdev-close-tray
+---
+
+Closes a block device's tray. If there is a block driver state tree associated
+with the block device (which is currently ejected), that tree will be loaded as
+the medium.
+
+Arguments:
+
+- device: block device name (json-string)
+
+Example (1):
+
+- { execute: blockdev-close-tray,
+ arguments: { device: ide1-cd0 } }
+
+- { timestamp: { seconds: 1418751345,
+microseconds: 272147 },
+ event: DEVICE_TRAY_MOVED,
+ data: { device: ide1-cd0,
+   tray-open: false } }
+
+- { return: {} }
+
+EQMP
+
+{
 .name   = query-named-block-nodes,
 .args_type  = ,
 .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes,
-- 
2.1.0




Re: [Qemu-devel] [PATCH 6/7] block/raw-posix: call plain fallocate in handle_aiocb_write_zeroes

2015-01-27 Thread Max Reitz

On 2015-01-27 at 13:19, Denis V. Lunev wrote:

On 27/01/15 20:57, Max Reitz wrote:

On 2015-01-27 at 08:51, Denis V. Lunev wrote:

There is a possibility that we are extending our image and thus writing
zeroes beyond the end of the file. In this case we do not need to care
about the hole to make sure that there is no data in the file under
this offset (pre-condition to fallocate(0) to work). We could simply 
call

fallocate(0).

This improves the performance of writing zeroes even on really old
platforms which do not have even FALLOC_FL_PUNCH_HOLE.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
  block/raw-posix.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c039bef..fa05239 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -60,7 +60,7 @@
  #define FS_NOCOW_FL 0x0080 /* Do not cow 
file */

  #endif
  #endif
-#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || 
defined(CONFIG_FALLOCATE_ZERO_RANGE)

+#ifdef CONFIG_FALLOCATE


This change doesn't seem right; CONFIG_FALLOCATE is set if 
posix_fallocate() is available, not for the Linux-specific 
fallocate() from linux/falloc.h.




here is a check for fallocate and posix_fallocate in configure script

# check for fallocate
fallocate=no
cat  $TMPC  EOF
#include fcntl.h

int main(void)
{
fallocate(0, 0, 0, 0);
return 0;
}
EOF
if compile_prog   ; then
  fallocate=yes
fi
...
# check for posix_fallocate
posix_fallocate=no
cat  $TMPC  EOF
#include fcntl.h

int main(void)
{
posix_fallocate(0, 0, 0);
return 0;
}
EOF
if compile_prog   ; then
posix_fallocate=yes
fi
...
if test $fallocate = yes ; then
  echo CONFIG_FALLOCATE=y  $config_host_mak
fi
...
if test $posix_fallocate = yes ; then
  echo CONFIG_POSIX_FALLOCATE=y  $config_host_mak
fi

Thus my check looks correct to me.


Oh, sorry, I somehow mixed those checks. You're right.

Very well then; maybe you want to mention this change in the commit 
message, though?


Max




  #include linux/falloc.h
  #endif
  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
@@ -902,7 +902,7 @@ static int translate_err(int err)
  return err;
  }
  -#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || 
defined(CONFIG_FALLOCATE_ZERO_RANGE)

+#ifdef CONFIG_FALLOCATE


Same here.


  static int do_fallocate(int fd, int mode, off_t offset, off_t len)
  {
  do {
@@ -981,6 +981,12 @@ static ssize_t 
handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)

  }
  #endif
  +#ifdef CONFIG_FALLOCATE
+if (aiocb-aio_offset = aiocb-bs-total_sectors  
BDRV_SECTOR_BITS) {
+return do_fallocate(s-fd, 0, aiocb-aio_offset, 
aiocb-aio_nbytes);

+}
+#endif
+


This seems fine though, but as I've asked in patch 5: Do we want to 
have a has_fallocate?


Other than that, this is the first usage of bs-total_sectors in this 
file; raw_co_get_block_status() does a similar check, but it uses 
bdrv_getlength() instead. If bs-total_sectors is correct, 
bdrv_getlength() will actually do nothing but return 
bs-total_sectors * BDRV_SECTOR_SIZE; it will only do more (that is, 
update bs-total_sectors) if it is not correct to use 
bs-total_sectors (and I feel like it may not be correct because 
BlockDriver.has_variable_length is true).


Max


ok, will do





Re: [Qemu-devel] [PATCH v1 1/2] target_arm: Remove memory region init from armv7m_init

2015-01-27 Thread Peter Maydell
On 24 January 2015 at 08:02, Alistair Francis alistai...@gmail.com wrote:
 This patch moves the memory region init code from the
 armv7m_init function to the stellaris_init function

 Signed-off-by: Alistair Francis alistai...@gmail.com
 Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 ---
 This has been split from the Netduino 2 machine patch
 series.

Reviewed-by: Peter Maydell peter.mayd...@linaro.org
and applied to target-arm.next.

PS: if you're sending more than one patch can you make sure you
include a cover letter email as well, please? It's a bit awkward
to track pending-review patches otherwise.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 02/50] iotests: Only create BB if necessary

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 Tests 071 and 081 test giving references in blockdev-add. It is not
 necessary to create a BlockBackend here, so omit it.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  tests/qemu-iotests/071 | 50 
 ++
  tests/qemu-iotests/071.out | 12 +++
  tests/qemu-iotests/081 | 14 -
  tests/qemu-iotests/081.out |  5 +++--
  4 files changed, 70 insertions(+), 11 deletions(-)
 

Reviewed-by: Eric Blake ebl...@redhat.com

but given my comments in 1/50, you'll want to also add a test somewhere
(either there or here) that omitting both id and node-name gives an error.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC v3 0/2] Add Generic PCI host device update

2015-01-27 Thread Peter Maydell
On 14 January 2015 at 10:16, Alvise Rigo a.r...@virtualopensystems.com wrote:
 This is just an update to address the comments received on v2.

 This patch series is based on the previous work [1] and [2] by Rob Herring.
 This is a stand alone work, but I will not hesitate to split the patches in a
 more accurate way if needed.

 TODO:
 - Add MSI, MSI-X support
 - PCI-E support. For this, you would probably look at Alexander Graf work [3]

So, my current belief is that Alex's patchset provides everything
this patch does and more (since it gives us PCI-E). Am I missing
something that means we need this patchset as well ?

thanks
-- PMM



Re: [Qemu-devel] [PATCH 04/50] hw/usb-storage: Check whether BB is inserted

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 Only call bdrv_key_required() on the BlockDriverState if the
 BlockBackend has an inserted medium.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  hw/usb/dev-storage.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake ebl...@redhat.com

 
 diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
 index 4539733..3123baf 100644
 --- a/hw/usb/dev-storage.c
 +++ b/hw/usb/dev-storage.c
 @@ -638,7 +638,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error 
 **errp)
  usb_msd_handle_reset(dev);
  s-scsi_dev = scsi_dev;
  
 -if (bdrv_key_required(blk_bs(blk))) {
 +if (blk_is_inserted(blk)  bdrv_key_required(blk_bs(blk))) {
  if (cur_mon) {
  monitor_read_bdrv_key_start(cur_mon, blk_bs(blk),
  usb_msd_password_cb, s);
 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH RESEND 42/50] blockdev: Implement eject with basic operations

2015-01-27 Thread Max Reitz
Implement 'eject' by calling blockdev-open-tray and
blockdev-remove-medium.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e4588b3..0b204eb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1865,15 +1865,15 @@ out:
 
 void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
 {
-BlockBackend *blk;
+Error *local_err = NULL;
 
-blk = blk_by_name(device);
-if (!blk) {
-error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+qmp_blockdev_open_tray(device, has_force, force, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
 
-eject_device(blk, force, errp);
+qmp_blockdev_remove_medium(device, errp);
 }
 
 void qmp_block_passwd(bool has_device, const char *device,
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 35/50] blockdev: Pull out blockdev option extraction

2015-01-27 Thread Max Reitz
Extract some of the blockdev option extraction code from blockdev_init()
into an own function. This simplifies blockdev_init() and will allow
reusing the code in a different function added in a follow-up patch.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 201 +
 1 file changed, 108 insertions(+), 93 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d99edbb..d573c5c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -341,24 +341,123 @@ static bool check_throttle_config(ThrottleConfig *cfg, 
Error **errp)
 
 typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType;
 
+static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
+ThrottleConfig *throttle_cfg, BlockdevDetectZeroesOptions *detect_zeroes,
+Error **errp)
+{
+const char *discard, *aio;
+Error *local_error = NULL;
+
+if (!qemu_opt_get_bool(opts, read-only, false)) {
+*bdrv_flags |= BDRV_O_RDWR;
+}
+if (qemu_opt_get_bool(opts, copy-on-read, false)) {
+*bdrv_flags |= BDRV_O_COPY_ON_READ;
+}
+
+if ((discard = qemu_opt_get(opts, discard)) != NULL) {
+if (bdrv_parse_discard_flags(discard, bdrv_flags) != 0) {
+error_setg(errp, Invalid discard option);
+return;
+}
+}
+
+if (qemu_opt_get_bool(opts, cache.writeback, true)) {
+*bdrv_flags |= BDRV_O_CACHE_WB;
+}
+if (qemu_opt_get_bool(opts, cache.direct, false)) {
+*bdrv_flags |= BDRV_O_NOCACHE;
+}
+if (qemu_opt_get_bool(opts, cache.no-flush, false)) {
+*bdrv_flags |= BDRV_O_NO_FLUSH;
+}
+
+#ifdef CONFIG_LINUX_AIO
+if ((aio = qemu_opt_get(opts, aio)) != NULL) {
+if (!strcmp(aio, native)) {
+*bdrv_flags |= BDRV_O_NATIVE_AIO;
+} else if (!strcmp(aio, threads)) {
+/* this is the default */
+} else {
+   error_setg(errp, invalid aio option);
+   return;
+}
+}
+#endif
+
+/* disk I/O throttling */
+memset(throttle_cfg, 0, sizeof(*throttle_cfg));
+throttle_cfg-buckets[THROTTLE_BPS_TOTAL].avg =
+qemu_opt_get_number(opts, throttling.bps-total, 0);
+throttle_cfg-buckets[THROTTLE_BPS_READ].avg  =
+qemu_opt_get_number(opts, throttling.bps-read, 0);
+throttle_cfg-buckets[THROTTLE_BPS_WRITE].avg =
+qemu_opt_get_number(opts, throttling.bps-write, 0);
+throttle_cfg-buckets[THROTTLE_OPS_TOTAL].avg =
+qemu_opt_get_number(opts, throttling.iops-total, 0);
+throttle_cfg-buckets[THROTTLE_OPS_READ].avg =
+qemu_opt_get_number(opts, throttling.iops-read, 0);
+throttle_cfg-buckets[THROTTLE_OPS_WRITE].avg =
+qemu_opt_get_number(opts, throttling.iops-write, 0);
+
+throttle_cfg-buckets[THROTTLE_BPS_TOTAL].max =
+qemu_opt_get_number(opts, throttling.bps-total-max, 0);
+throttle_cfg-buckets[THROTTLE_BPS_READ].max  =
+qemu_opt_get_number(opts, throttling.bps-read-max, 0);
+throttle_cfg-buckets[THROTTLE_BPS_WRITE].max =
+qemu_opt_get_number(opts, throttling.bps-write-max, 0);
+throttle_cfg-buckets[THROTTLE_OPS_TOTAL].max =
+qemu_opt_get_number(opts, throttling.iops-total-max, 0);
+throttle_cfg-buckets[THROTTLE_OPS_READ].max =
+qemu_opt_get_number(opts, throttling.iops-read-max, 0);
+throttle_cfg-buckets[THROTTLE_OPS_WRITE].max =
+qemu_opt_get_number(opts, throttling.iops-write-max, 0);
+
+throttle_cfg-op_size =
+qemu_opt_get_number(opts, throttling.iops-size, 0);
+
+if (!check_throttle_config(throttle_cfg, errp)) {
+return;
+}
+
+*detect_zeroes =
+qapi_enum_parse(BlockdevDetectZeroesOptions_lookup,
+qemu_opt_get(opts, detect-zeroes),
+BLOCKDEV_DETECT_ZEROES_OPTIONS_MAX,
+BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
+local_error);
+if (local_error) {
+error_propagate(errp, local_error);
+return;
+}
+
+if (*detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP 
+!(*bdrv_flags  BDRV_O_UNMAP))
+{
+error_setg(errp, setting detect-zeroes to unmap is not allowed 
+ without setting discard operation to unmap);
+return;
+}
+
+}
+
 /* Takes the ownership of bs_opts */
 static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
Error **errp)
 {
 const char *buf;
-int ro = 0;
 int bdrv_flags = 0;
 int on_read_error, on_write_error;
 BlockBackend *blk;
 BlockDriverState *bs;
 ThrottleConfig cfg;
 int snapshot = 0;
-bool copy_on_read;
 Error *error = NULL;
 QemuOpts *opts;
 const char *id;
 bool has_driver_specific_opts;
-BlockdevDetectZeroesOptions detect_zeroes;
+BlockdevDetectZeroesOptions detect_zeroes =
+BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
 
 /* Check common options 

Re: [Qemu-devel] [PATCH 7/7] block/raw-posix: set max_write_zeroes to INT_MAX for regular files

2015-01-27 Thread Max Reitz

On 2015-01-27 at 08:51, Denis V. Lunev wrote:

fallocate() works fine and could handle properly with arbitrary size
requests.


Maybe could properly handle arbitrary size requests (or 
...arbitrarily sized requests)?



There is no sense to reduce the amount of space to fallocate.
The bigger is the size, the better is the performance as the amount of
journal updates is reduced.


True for fallocate(). But is it true for xfs_write_zeroes(), too? I 
guess so, but I don't know.


If it does, the patch looks good to me.

Max


Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
  block/raw-posix.c | 17 +
  1 file changed, 17 insertions(+)




Re: [Qemu-devel] [PATCH 6/7] block/raw-posix: call plain fallocate in handle_aiocb_write_zeroes

2015-01-27 Thread Denis V. Lunev

On 27/01/15 20:57, Max Reitz wrote:

On 2015-01-27 at 08:51, Denis V. Lunev wrote:

There is a possibility that we are extending our image and thus writing
zeroes beyond the end of the file. In this case we do not need to care
about the hole to make sure that there is no data in the file under
this offset (pre-condition to fallocate(0) to work). We could simply 
call

fallocate(0).

This improves the performance of writing zeroes even on really old
platforms which do not have even FALLOC_FL_PUNCH_HOLE.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
  block/raw-posix.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c039bef..fa05239 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -60,7 +60,7 @@
  #define FS_NOCOW_FL 0x0080 /* Do not cow 
file */

  #endif
  #endif
-#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || 
defined(CONFIG_FALLOCATE_ZERO_RANGE)

+#ifdef CONFIG_FALLOCATE


This change doesn't seem right; CONFIG_FALLOCATE is set if 
posix_fallocate() is available, not for the Linux-specific fallocate() 
from linux/falloc.h.




here is a check for fallocate and posix_fallocate in configure script

# check for fallocate
fallocate=no
cat  $TMPC  EOF
#include fcntl.h

int main(void)
{
fallocate(0, 0, 0, 0);
return 0;
}
EOF
if compile_prog   ; then
  fallocate=yes
fi
...
# check for posix_fallocate
posix_fallocate=no
cat  $TMPC  EOF
#include fcntl.h

int main(void)
{
posix_fallocate(0, 0, 0);
return 0;
}
EOF
if compile_prog   ; then
posix_fallocate=yes
fi
...
if test $fallocate = yes ; then
  echo CONFIG_FALLOCATE=y  $config_host_mak
fi
...
if test $posix_fallocate = yes ; then
  echo CONFIG_POSIX_FALLOCATE=y  $config_host_mak
fi

Thus my check looks correct to me.


  #include linux/falloc.h
  #endif
  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
@@ -902,7 +902,7 @@ static int translate_err(int err)
  return err;
  }
  -#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || 
defined(CONFIG_FALLOCATE_ZERO_RANGE)

+#ifdef CONFIG_FALLOCATE


Same here.


  static int do_fallocate(int fd, int mode, off_t offset, off_t len)
  {
  do {
@@ -981,6 +981,12 @@ static ssize_t 
handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)

  }
  #endif
  +#ifdef CONFIG_FALLOCATE
+if (aiocb-aio_offset = aiocb-bs-total_sectors  
BDRV_SECTOR_BITS) {
+return do_fallocate(s-fd, 0, aiocb-aio_offset, 
aiocb-aio_nbytes);

+}
+#endif
+


This seems fine though, but as I've asked in patch 5: Do we want to 
have a has_fallocate?


Other than that, this is the first usage of bs-total_sectors in this 
file; raw_co_get_block_status() does a similar check, but it uses 
bdrv_getlength() instead. If bs-total_sectors is correct, 
bdrv_getlength() will actually do nothing but return bs-total_sectors 
* BDRV_SECTOR_SIZE; it will only do more (that is, update 
bs-total_sectors) if it is not correct to use bs-total_sectors (and 
I feel like it may not be correct because 
BlockDriver.has_variable_length is true).


Max


ok, will do



Re: [Qemu-devel] [PATCH 6/7] block/raw-posix: call plain fallocate in handle_aiocb_write_zeroes

2015-01-27 Thread Denis V. Lunev

On 27/01/15 21:24, Max Reitz wrote:

On 2015-01-27 at 13:19, Denis V. Lunev wrote:

On 27/01/15 20:57, Max Reitz wrote:

On 2015-01-27 at 08:51, Denis V. Lunev wrote:
There is a possibility that we are extending our image and thus 
writing

zeroes beyond the end of the file. In this case we do not need to care
about the hole to make sure that there is no data in the file under
this offset (pre-condition to fallocate(0) to work). We could 
simply call

fallocate(0).

This improves the performance of writing zeroes even on really old
platforms which do not have even FALLOC_FL_PUNCH_HOLE.

Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Kevin Wolf kw...@redhat.com
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Peter Lieven p...@kamp.de
CC: Fam Zheng f...@redhat.com
---
  block/raw-posix.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index c039bef..fa05239 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -60,7 +60,7 @@
  #define FS_NOCOW_FL 0x0080 /* Do not cow 
file */

  #endif
  #endif
-#if defined(CONFIG_FALLOCATE_PUNCH_HOLE) || 
defined(CONFIG_FALLOCATE_ZERO_RANGE)

+#ifdef CONFIG_FALLOCATE


This change doesn't seem right; CONFIG_FALLOCATE is set if 
posix_fallocate() is available, not for the Linux-specific 
fallocate() from linux/falloc.h.




here is a check for fallocate and posix_fallocate in configure script

# check for fallocate
fallocate=no
cat  $TMPC  EOF
#include fcntl.h

int main(void)
{
fallocate(0, 0, 0, 0);
return 0;
}
EOF
if compile_prog   ; then
  fallocate=yes
fi
...
# check for posix_fallocate
posix_fallocate=no
cat  $TMPC  EOF
#include fcntl.h

int main(void)
{
posix_fallocate(0, 0, 0);
return 0;
}
EOF
if compile_prog   ; then
posix_fallocate=yes
fi
...
if test $fallocate = yes ; then
  echo CONFIG_FALLOCATE=y  $config_host_mak
fi
...
if test $posix_fallocate = yes ; then
  echo CONFIG_POSIX_FALLOCATE=y  $config_host_mak
fi

Thus my check looks correct to me.


Oh, sorry, I somehow mixed those checks. You're right.

Very well then; maybe you want to mention this change in the commit 
message, though?


Max


no prob



Re: [Qemu-devel] [PATCH V2 4/4] target-arm: Add missing SP_ELx register definition

2015-01-27 Thread Peter Maydell
On 23 January 2015 at 16:17, Greg Bellows greg.bell...@linaro.org wrote:
 Added CP register definitions for SP_EL1 and SP_EL2.

 Signed-off-by: Greg Bellows greg.bell...@linaro.org
 Reviewed-by: Peter Maydell peter.mayd...@linaro.org

 ---

 v1 - v2
 - Remove unnecessary accessfn for SP_EL1/2
 - Revert SP_EL0 accessfn name to sp_el0_access
 ---
  target-arm/helper.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/target-arm/helper.c b/target-arm/helper.c
 index 29f3b62..79c54a9 100644
 --- a/target-arm/helper.c
 +++ b/target-arm/helper.c
 @@ -2329,6 +2329,10 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.access = PL1_RW, .accessfn = sp_el0_access,
.type = ARM_CP_NO_MIGRATE,
.fieldoffset = offsetof(CPUARMState, sp_el[0]) },
 +{ .name = SP_EL1, .state = ARM_CP_STATE_AA64,
 +  .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 1, .opc2 = 0,
 +  .access = PL2_RW, .type = ARM_CP_NO_MIGRATE,
 +  .fieldoffset = offsetof(CPUARMState, sp_el[1]) },
  { .name = SPSel, .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 4, .crm = 2, .opc2 = 0,
.type = ARM_CP_NO_MIGRATE,
 @@ -2410,6 +2414,10 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
.access = PL2_RW, .writefn = vbar_write,
.fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]),
.resetvalue = 0 },
 +{ .name = SP_EL2, .state = ARM_CP_STATE_AA64,
 +  .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 1, .opc2 = 0,
 +  .access = PL3_RW, .type = ARM_CP_NO_MIGRATE,
 +  .fieldoffset = offsetof(CPUARMState, sp_el[2]) },
  REGINFO_SENTINEL
  };

As I was assembling my target-arm queue I found that this patch
and the 'split ARM_CP_NO_MIGRATE' patch semantically conflict;
since this patch happened to be earlier in the queue than that
one, I've resolved this by adding changes to the 'split' patch
which change these ARM_CP_NO_MIGRATE uses to ARM_CP_ALIAS, in
line with how we handled the SP_EL0 regdef.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 06/50] block: Add blk_is_available()

2015-01-27 Thread Eric Blake
On 01/26/2015 09:02 AM, Max Reitz wrote:
 blk_is_available() returns true iff the BDS is inserted (which means
 blk_bs() is not NULL and bdrv_is_inserted() returns true) and if the
 tray of the guest device is closed.
 
 blk_is_inserted() is changed to return true only if blk_bs() is not
 NULL.
 
 Signed-off-by: Max Reitz mre...@redhat.com
 ---
  block/block-backend.c  | 7 ++-
  include/sysemu/block-backend.h | 1 +
  2 files changed, 7 insertions(+), 1 deletion(-)
 

 +++ b/include/sysemu/block-backend.h
 @@ -127,6 +127,7 @@ int blk_enable_write_cache(BlockBackend *blk);
  void blk_set_enable_write_cache(BlockBackend *blk, bool wce);
  void blk_invalidate_cache(BlockBackend *blk, Error **errp);
  int blk_is_inserted(BlockBackend *blk);

Should we change blk_is_inserted to return bool?

 +bool blk_is_available(BlockBackend *blk);

Looks reasonable.
Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH RESEND 34/50] blockdev: Do not create BDS for empty drive

2015-01-27 Thread Max Reitz
Do not use rudimentary BDSs for empty drives any longer (for
freshly created drives).

With this change, bdrv_close_all() has no effect on empty drives (whose
media were not changed) any longer. This breaks some test outputs, fix
them.

After a follow-up patch, empty drives will generally use a NULL BDS, not
only the freshly created drives.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c | 68 ++
 tests/qemu-iotests/067.out | 44 --
 tests/qemu-iotests/071.out |  2 --
 tests/qemu-iotests/081.out |  1 -
 tests/qemu-iotests/087.out |  5 
 5 files changed, 38 insertions(+), 82 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index f82b20c..d99edbb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -502,16 +502,40 @@ static BlockBackend *blockdev_init(const char *file, 
QDict *bs_opts,
 goto early_err;
 }
 
+if (snapshot) {
+/* always use cache=unsafe with snapshot */
+bdrv_flags = ~BDRV_O_CACHE_MASK;
+bdrv_flags |= (BDRV_O_SNAPSHOT|BDRV_O_CACHE_WB|BDRV_O_NO_FLUSH);
+}
+
+if (copy_on_read) {
+bdrv_flags |= BDRV_O_COPY_ON_READ;
+}
+
+if (runstate_check(RUN_STATE_INMIGRATE)) {
+bdrv_flags |= BDRV_O_INCOMING;
+}
+
+bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
+
 /* init */
 if ((!file || !*file)  !has_driver_specific_opts) {
-blk = blk_new_with_bs(qemu_opts_id(opts), errp);
+BlockBackendRootState *blk_rs;
+
+blk = blk_new(qemu_opts_id(opts), errp);
 if (!blk) {
 goto early_err;
 }
 
-bs = blk_bs(blk);
-bs-open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
-bs-read_only = ro;
+blk_rs = blk_get_root_state(blk);
+blk_rs-open_flags= bdrv_flags;
+blk_rs-read_only = ro;
+blk_rs-detect_zeroes = detect_zeroes;
+
+if (throttle_enabled(cfg)) {
+blk_rs-io_limits_enabled = true;
+blk_rs-throttle_config = cfg;
+}
 
 QDECREF(bs_opts);
 } else {
@@ -519,43 +543,27 @@ static BlockBackend *blockdev_init(const char *file, 
QDict *bs_opts,
 file = NULL;
 }
 
-if (snapshot) {
-/* always use cache=unsafe with snapshot */
-bdrv_flags = ~BDRV_O_CACHE_MASK;
-bdrv_flags |= (BDRV_O_SNAPSHOT|BDRV_O_CACHE_WB|BDRV_O_NO_FLUSH);
-}
-
-if (copy_on_read) {
-bdrv_flags |= BDRV_O_COPY_ON_READ;
-}
-
-if (runstate_check(RUN_STATE_INMIGRATE)) {
-bdrv_flags |= BDRV_O_INCOMING;
-}
-
-bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
-
 blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags,
errp);
 if (!blk) {
 goto err_no_bs_opts;
 }
 bs = blk_bs(blk);
-}
 
-bs-detect_zeroes = detect_zeroes;
+bs-detect_zeroes = detect_zeroes;
 
-blk_set_on_error(blk, on_read_error, on_write_error);
+/* disk I/O throttling */
+if (throttle_enabled(cfg)) {
+bdrv_io_limits_enable(bs);
+bdrv_set_io_limits(bs, cfg);
+}
 
-/* disk I/O throttling */
-if (throttle_enabled(cfg)) {
-bdrv_io_limits_enable(bs);
-bdrv_set_io_limits(bs, cfg);
+if (bdrv_key_required(bs)) {
+autostart = 0;
+}
 }
 
-if (bdrv_key_required(bs)) {
-autostart = 0;
-}
+blk_set_on_error(blk, on_read_error, on_write_error);
 
 err_no_bs_opts:
 qemu_opts_del(opts);
diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
index 42bae32..47b0b68 100644
--- a/tests/qemu-iotests/067.out
+++ b/tests/qemu-iotests/067.out
@@ -154,17 +154,6 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti
 },
 event: SHUTDOWN
 }
-{
-timestamp: {
-seconds:  TIMESTAMP,
-microseconds:  TIMESTAMP
-},
-event: DEVICE_TRAY_MOVED,
-data: {
-device: ide1-cd0,
-tray-open: true
-}
-}
 
 
 === -drive/device_add and device_del ===
@@ -324,17 +313,6 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk
 },
 event: SHUTDOWN
 }
-{
-timestamp: {
-seconds:  TIMESTAMP,
-microseconds:  TIMESTAMP
-},
-event: DEVICE_TRAY_MOVED,
-data: {
-device: ide1-cd0,
-tray-open: true
-}
-}
 
 
 === drive_add/device_add and device_del ===
@@ -497,17 +475,6 @@ Testing:
 },
 event: SHUTDOWN
 }
-{
-timestamp: {
-seconds:  TIMESTAMP,
-microseconds:  TIMESTAMP
-},
-event: DEVICE_TRAY_MOVED,
-data: {
-device: ide1-cd0,
-tray-open: true
-}
-}
 
 
 === blockdev_add/device_add and device_del ===
@@ -716,16 +683,5 @@ Testing:
 },
 event: SHUTDOWN
 }
-{
-timestamp: {
-seconds:  TIMESTAMP,
-microseconds:  TIMESTAMP
-},
-

[Qemu-devel] [PATCH RESEND 10/50] block: Remove wr_highest_offset from BlockAcctStats

2015-01-27 Thread Max Reitz
BlockAcctStats contains statistics about the data transferred from and
to the device; wr_highest_offset does not fit in with the rest.

Furthermore, those statistics are supposed to be specific for a certain
device and not necessarily for a BDS (see the comment above
bdrv_get_stats()); on the other hand, wr_highest_offset may be a rather
important information to know for each BDS. When BlockAcctStats is
finally removed from the BDS, we will want to keep wr_highest_offset in
the BDS.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block.c| 4 +++-
 block/accounting.c | 9 -
 block/qapi.c   | 4 ++--
 include/block/accounting.h | 3 ---
 include/block/block_int.h  | 3 +++
 5 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index eff92ca..5db71c6 100644
--- a/block.c
+++ b/block.c
@@ -3312,7 +3312,9 @@ static int coroutine_fn 
bdrv_aligned_pwritev(BlockDriverState *bs,
 
 bdrv_set_dirty(bs, sector_num, nb_sectors);
 
-block_acct_highest_sector(bs-stats, sector_num, nb_sectors);
+if (bs-wr_highest_sector  sector_num + nb_sectors - 1) {
+bs-wr_highest_sector = sector_num + nb_sectors - 1;
+}
 
 if (ret = 0) {
 bs-total_sectors = MAX(bs-total_sectors, sector_num + nb_sectors);
diff --git a/block/accounting.c b/block/accounting.c
index 18102f0..c77b6c2 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -45,12 +45,3 @@ void block_acct_done(BlockAcctStats *stats, BlockAcctCookie 
*cookie)
 stats-total_time_ns[cookie-type] +=
 qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - cookie-start_time_ns;
 }
-
-
-void block_acct_highest_sector(BlockAcctStats *stats, int64_t sector_num,
-   unsigned int nb_sectors)
-{
-if (stats-wr_highest_sector  sector_num + nb_sectors - 1) {
-stats-wr_highest_sector = sector_num + nb_sectors - 1;
-}
-}
diff --git a/block/qapi.c b/block/qapi.c
index 8c3b9d9..4e97574 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -338,13 +338,13 @@ static BlockStats *bdrv_query_stats(const 
BlockDriverState *bs,
 s-stats-wr_bytes = bs-stats.nr_bytes[BLOCK_ACCT_WRITE];
 s-stats-rd_operations = bs-stats.nr_ops[BLOCK_ACCT_READ];
 s-stats-wr_operations = bs-stats.nr_ops[BLOCK_ACCT_WRITE];
-s-stats-wr_highest_offset =
-bs-stats.wr_highest_sector * BDRV_SECTOR_SIZE;
 s-stats-flush_operations = bs-stats.nr_ops[BLOCK_ACCT_FLUSH];
 s-stats-wr_total_time_ns = bs-stats.total_time_ns[BLOCK_ACCT_WRITE];
 s-stats-rd_total_time_ns = bs-stats.total_time_ns[BLOCK_ACCT_READ];
 s-stats-flush_total_time_ns = bs-stats.total_time_ns[BLOCK_ACCT_FLUSH];
 
+s-stats-wr_highest_offset = bs-wr_highest_sector * BDRV_SECTOR_SIZE;
+
 if (bs-file) {
 s-has_parent = true;
 s-parent = bdrv_query_stats(bs-file, query_backing);
diff --git a/include/block/accounting.h b/include/block/accounting.h
index 50b42b3..9089b67 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -39,7 +39,6 @@ typedef struct BlockAcctStats {
 uint64_t nr_bytes[BLOCK_MAX_IOTYPE];
 uint64_t nr_ops[BLOCK_MAX_IOTYPE];
 uint64_t total_time_ns[BLOCK_MAX_IOTYPE];
-uint64_t wr_highest_sector;
 } BlockAcctStats;
 
 typedef struct BlockAcctCookie {
@@ -51,7 +50,5 @@ typedef struct BlockAcctCookie {
 void block_acct_start(BlockAcctStats *stats, BlockAcctCookie *cookie,
   int64_t bytes, enum BlockAcctType type);
 void block_acct_done(BlockAcctStats *stats, BlockAcctCookie *cookie);
-void block_acct_highest_sector(BlockAcctStats *stats, int64_t sector_num,
-   unsigned int nb_sectors);
 
 #endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index c6ab73a..e309d8a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -366,6 +366,9 @@ struct BlockDriverState {
 /* I/O stats (display with info blockstats). */
 BlockAcctStats stats;
 
+/* Highest sector index written to */
+uint64_t wr_highest_sector;
+
 /* I/O Limits */
 BlockLimits bl;
 
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 18/50] block: Respect empty BB in bdrv_query_info()

2015-01-27 Thread Max Reitz
Signed-off-by: Max Reitz mre...@redhat.com
---
 block/qapi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 9a44d40..db42a6e 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -282,12 +282,12 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo 
**p_info,
 info-io_status = blk_iostatus(blk);
 }
 
-if (!QLIST_EMPTY(bs-dirty_bitmaps)) {
+if (bs  !QLIST_EMPTY(bs-dirty_bitmaps)) {
 info-has_dirty_bitmaps = true;
 info-dirty_bitmaps = bdrv_query_dirty_bitmaps(bs);
 }
 
-if (bs-drv) {
+if (bs  bs-drv) {
 info-has_inserted = true;
 info-inserted = bdrv_block_device_info(bs);
 
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 31/50] block: Add blk_insert_bs()

2015-01-27 Thread Max Reitz
This function associates the given BlockDriverState with the given
BlockBackend.

Signed-off-by: Max Reitz mre...@redhat.com
---
 block/block-backend.c  | 16 
 include/sysemu/block-backend.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 760558f..656ebfc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -312,6 +312,22 @@ void blk_hide_on_behalf_of_do_drive_del(BlockBackend *blk)
 }
 
 /*
+ * Associates a new BlockDriverState with @blk.
+ */
+void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
+{
+if (bs-blk == blk) {
+return;
+}
+
+assert(!blk-bs);
+assert(!bs-blk);
+bdrv_ref(bs);
+blk-bs = bs;
+bs-blk = blk;
+}
+
+/*
  * Attach device model @dev to @blk.
  * Return 0 on success, -EBUSY when a device model is attached already.
  */
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index afb62e1..b6cf5bf 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -72,6 +72,7 @@ BlockBackend *blk_by_name(const char *name);
 BlockBackend *blk_next(BlockBackend *blk);
 
 BlockDriverState *blk_bs(BlockBackend *blk);
+void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
 
 void blk_hide_on_behalf_of_do_drive_del(BlockBackend *blk);
 
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 03/50] hw/block/fdc: Implement tray status

2015-01-27 Thread Max Reitz
The tray of an FDD is open iff there is no medium inserted (there are
only two states for an FDD: medium inserted or no medium inserted).

This results in the tray being reported as open if qemu has been started
with the default floppy drive, which breaks some tests. Fix them.

Signed-off-by: Max Reitz mre...@redhat.com
---
 hw/block/fdc.c | 20 +---
 tests/fdc-test.c   |  4 +---
 tests/qemu-iotests/067.out | 60 +++---
 tests/qemu-iotests/071.out |  2 --
 tests/qemu-iotests/081.out |  1 -
 tests/qemu-iotests/087.out |  5 
 6 files changed, 26 insertions(+), 66 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 2bf87c9..0c5a6b4 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -192,6 +192,8 @@ typedef struct FDrive {
 uint8_t ro;   /* Is read-only   */
 uint8_t media_changed;/* Is media changed   */
 uint8_t media_rate;   /* Data rate of medium*/
+
+bool media_inserted;  /* Is there a medium in the tray */
 } FDrive;
 
 static void fd_init(FDrive *drv)
@@ -261,7 +263,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t 
track, uint8_t sect,
 #endif
 drv-head = head;
 if (drv-track != track) {
-if (drv-blk != NULL  blk_is_inserted(drv-blk)) {
+if (drv-media_inserted) {
 drv-media_changed = 0;
 }
 ret = 1;
@@ -270,7 +272,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t 
track, uint8_t sect,
 drv-sect = sect;
 }
 
-if (drv-blk == NULL || !blk_is_inserted(drv-blk)) {
+if (!drv-media_inserted) {
 ret = 2;
 }
 
@@ -296,7 +298,7 @@ static void fd_revalidate(FDrive *drv)
 ro = blk_is_read_only(drv-blk);
 pick_geometry(drv-blk, nb_heads, max_track,
   last_sect, drv-drive, drive, rate);
-if (!blk_is_inserted(drv-blk)) {
+if (!drv-media_inserted) {
 FLOPPY_DPRINTF(No disk in drive\n);
 } else {
 FLOPPY_DPRINTF(Floppy disk (%d h %d t %d s) %s\n, nb_heads,
@@ -2062,12 +2064,21 @@ static void fdctrl_change_cb(void *opaque, bool load)
 {
 FDrive *drive = opaque;
 
+drive-media_inserted = load  drive-blk  blk_is_inserted(drive-blk);
+
 drive-media_changed = 1;
 fd_revalidate(drive);
 }
 
+static bool fdctrl_is_tray_open(void *opaque)
+{
+FDrive *drive = opaque;
+return !drive-media_inserted;
+}
+
 static const BlockDevOps fdctrl_block_ops = {
 .change_media_cb = fdctrl_change_cb,
+.is_tray_open = fdctrl_is_tray_open,
 };
 
 /* Init functions */
@@ -2095,6 +2106,9 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error 
**errp)
 fdctrl_change_cb(drive, 0);
 if (drive-blk) {
 blk_set_dev_ops(drive-blk, fdctrl_block_ops, drive);
+if (blk_is_inserted(drive-blk)) {
+drive-media_inserted = true;
+}
 }
 }
 }
diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 3c6c83c..f287c10 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -293,9 +293,7 @@ static void test_media_insert(void)
 qmp_discard_response({'execute':'change', 'arguments':{
   'device':'floppy0', 'target': %s, 'arg': 'raw' }},
  test_image);
-qmp_discard_response(); /* ignore event
- (FIXME open - open transition?!) */
-qmp_discard_response(); /* ignore event */
+qmp_discard_response(); /* ignore event (open - close) */
 
 dir = inb(FLOPPY_BASE + reg_dir);
 assert_bit_set(dir, DSKCHG);
diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
index 00b3eae..42bae32 100644
--- a/tests/qemu-iotests/067.out
+++ b/tests/qemu-iotests/067.out
@@ -69,7 +69,7 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti
 device: floppy0,
 locked: false,
 removable: true,
-tray_open: false,
+tray_open: true,
 type: unknown
 },
 {
@@ -131,7 +131,7 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti
 device: floppy0,
 locked: false,
 removable: true,
-tray_open: false,
+tray_open: true,
 type: unknown
 },
 {
@@ -165,17 +165,6 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti
 tray-open: true
 }
 }
-{
-timestamp: {
-seconds:  TIMESTAMP,
-microseconds:  TIMESTAMP
-},
-event: DEVICE_TRAY_MOVED,
-data: {
-device: floppy0,
-tray-open: true
-}
-}
 
 
 === -drive/device_add and device_del ===
@@ -246,7 +235,7 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk
 device: floppy0,
 locked: false,
 removable: true,
-  

[Qemu-devel] [PATCH RESEND 48/50] hmp: Add read-only option to change command

2015-01-27 Thread Max Reitz
Expose the new read-only option of 'blockdev-change-medium' for the
'change' HMP command.

Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 hmp-commands.hx | 20 +---
 hmp.c   | 21 -
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..70cb7d7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -196,8 +196,8 @@ ETEXI
 
 {
 .name   = change,
-.args_type  = device:B,target:F,arg:s?,
-.params = device filename [format],
+.args_type  = device:B,target:F,arg:s?,read-only:s?,
+.params = device filename [format [read-only]],
 .help   = change a removable medium, optional format,
 .mhandler.cmd = hmp_change,
 },
@@ -209,7 +209,7 @@ STEXI
 Change the configuration of a device.
 
 @table @option
-@item change @var{diskdevice} @var{filename} [@var{format}]
+@item change @var{diskdevice} @var{filename} [@var{format} [@var{read-only}]]
 Change the medium for a removable disk device to point to @var{filename}. eg
 
 @example
@@ -218,6 +218,20 @@ Change the medium for a removable disk device to point to 
@var{filename}. eg
 
 @var{format} is optional.
 
+@var{read-only} may be used to change the read-only status of the device. It
+accepts the following values:
+
+@table @var
+@item retain
+Retains the current status; this is the default.
+
+@item ro
+Makes the device read-only.
+
+@item rw
+Makes the device writable.
+@end table
+
 @item change vnc @var{display},@var{options}
 Change the configuration of the VNC server. The valid syntax for @var{display}
 and @var{options} are described at @ref{sec_invocation}. eg
diff --git a/hmp.c b/hmp.c
index dbf0947..8e75771 100644
--- a/hmp.c
+++ b/hmp.c
@@ -24,6 +24,7 @@
 #include monitor/monitor.h
 #include qapi/opts-visitor.h
 #include qapi/string-output-visitor.h
+#include qapi/util.h
 #include qapi-visit.h
 #include ui/console.h
 #include block/qapi.h
@@ -1178,9 +1179,15 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 const char *device = qdict_get_str(qdict, device);
 const char *target = qdict_get_str(qdict, target);
 const char *arg = qdict_get_try_str(qdict, arg);
+const char *read_only = qdict_get_try_str(qdict, read-only);
+BlockdevChangeReadOnlyMode read_only_mode = 0;
 Error *err = NULL;
 
 if (strcmp(device, vnc) == 0) {
+if (read_only) {
+monitor_printf(mon, Parameter 'read-only' is invalid for VNC);
+return;
+}
 if (strcmp(target, passwd) == 0 ||
 strcmp(target, password) == 0) {
 if (!arg) {
@@ -1190,7 +1197,19 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 }
 qmp_change(vnc, target, !!arg, arg, err);
 } else {
-qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, err);
+if (read_only) {
+read_only_mode =
+qapi_enum_parse(BlockdevChangeReadOnlyMode_lookup,
+read_only, BLOCKDEV_CHANGE_READ_ONLY_MODE_MAX,
+BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, err);
+if (err) {
+hmp_handle_error(mon, err);
+return;
+}
+}
+
+qmp_blockdev_change_medium(device, target, !!arg, arg,
+   !!read_only, read_only_mode, err);
 if (err 
 error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
 error_free(err);
-- 
2.1.0




[Qemu-devel] [PATCH RESEND 17/50] block: Respect empty BB in bdrv_lookup_bs()

2015-01-27 Thread Max Reitz
blk_by_name() may return a BlockBackend for which blk_bs() returns NULL.
In this case, an error should be returned (instead of just returning
NULL without modifying *errp).

Signed-off-by: Max Reitz mre...@redhat.com
---
 block.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block.c b/block.c
index 9a0a510..b7e631c 100644
--- a/block.c
+++ b/block.c
@@ -3718,6 +3718,11 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
 blk = blk_by_name(device);
 
 if (blk) {
+if (!blk_bs(blk)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+return NULL;
+}
+
 return blk_bs(blk);
 }
 }
-- 
2.1.0




Re: [Qemu-devel] [PATCH 2/2] bootdevice: update boot_order in MachineState

2015-01-27 Thread Dinar Valeev
On 01/27/2015 03:51 AM, Gonglei wrote:
 On 2015/1/27 7:52, dval...@suse.de wrote:
 
 From: Dinar Valeev dval...@suse.com

 on sPAPR we need to update boot_order in MachineState in case it
 got changed on reset.

 Signed-off-by: Dinar Valeev dval...@suse.com
 ---
  bootdevice.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/bootdevice.c b/bootdevice.c
 index 5914417..4f11a06 100644
 --- a/bootdevice.c
 +++ b/bootdevice.c
 @@ -26,6 +26,7 @@
  #include qapi/visitor.h
  #include qemu/error-report.h
  #include hw/hw.h
 +#include hw/boards.h
  
  typedef struct FWBootEntry FWBootEntry;
  
 @@ -50,6 +51,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void 
 *opaque)
  void qemu_boot_set(const char *boot_order, Error **errp)
  {
  Error *local_err = NULL;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +machine-boot_order = boot_order;
  
  if (!boot_set_handler) {
  error_setg(errp, no function defined to set boot device list for
 
 Have you registered boot set handler on ppc/sPAPR platform by calling
 qemu_register_boot_set()? Otherwise qemu_boot_set function
  will return error.
No, I set boot_order on each machine reset. My tests are showing it works 
without an error.
Previous patch was using qemu_register_boot_set, but Alexender Graf, suggested 
me to use MachineState and simply update on each guest reset.

 
 Regards,
 -Gonglei
 




Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine

2015-01-27 Thread Claudio Fontana
On 22.01.2015 16:52, Alexander Graf wrote:
 
 
 On 22.01.15 16:28, Claudio Fontana wrote:
 Hi Alexander,

 thank you for the respin. I retested with the new mappings on OSv for 
 AArch64,
 and they show up ok in the guest.

 Just a couple minor comments below, otherwise I think this is good.

 On 21.01.2015 17:18, Alexander Graf wrote:
 Now that we have a working generic PCIe host bridge driver, we can plug
 it into ARMs virt machine to always have PCIe available to normal ARM VMs.

 I've successfully managed to expose a Bochs VGA device, XHCI and an e1000
 into an AArch64 VM with this and they all lived happily ever after.

 Signed-off-by: Alexander Graf ag...@suse.de
 Tested-by: Claudio Fontana claudio.font...@huawei.com

 ---

 Linux 3.19 only supports the generic PCIe host bridge driver for 32bit ARM
 systems. If you want to use it with AArch64 guests, please apply the 
 following
 patch or wait until upstream cleaned up the code properly:

   http://csgraf.de/agraf/pci/pci-3.19.patch

 v1 - v2:

   - Add define for pci range types
   - Remove mmio_window_size
   - Use 4 PCI INTX IRQ lines
 ---
  default-configs/arm-softmmu.mak |   2 +
  hw/arm/virt.c   | 112 
 ++--
  include/sysemu/device_tree.h|   9 
  3 files changed, 118 insertions(+), 5 deletions(-)

 diff --git a/default-configs/arm-softmmu.mak 
 b/default-configs/arm-softmmu.mak
 index f3513fa..7671ee2 100644
 --- a/default-configs/arm-softmmu.mak
 +++ b/default-configs/arm-softmmu.mak
 @@ -82,6 +82,8 @@ CONFIG_ZYNQ=y
  CONFIG_VERSATILE_PCI=y
  CONFIG_VERSATILE_I2C=y
  
 +CONFIG_PCI_GENERIC=y
 +
  CONFIG_SDHCI=y
  CONFIG_INTEGRATOR_DEBUG=y
  
 diff --git a/hw/arm/virt.c b/hw/arm/virt.c
 index 2353440..a727b4e 100644
 --- a/hw/arm/virt.c
 +++ b/hw/arm/virt.c
 @@ -42,6 +42,7 @@
  #include exec/address-spaces.h
  #include qemu/bitops.h
  #include qemu/error-report.h
 +#include hw/pci-host/gpex.h
  
  #define NUM_VIRTIO_TRANSPORTS 32
  
 @@ -69,6 +70,7 @@ enum {
  VIRT_MMIO,
  VIRT_RTC,
  VIRT_FW_CFG,
 +VIRT_PCIE,
  };
  
  typedef struct MemMapEntry {
 @@ -129,13 +131,14 @@ static const MemMapEntry a15memmap[] = {
  [VIRT_FW_CFG] = { 0x0902, 0x000a },
  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that 
 size */
 -/* 0x1000 .. 0x4000 reserved for PCI */
 +[VIRT_PCIE] =   { 0x1000, 0x3000 },
  [VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 },
  };
  
  static const int a15irqmap[] = {
  [VIRT_UART] = 1,
  [VIRT_RTC] = 2,
 +[VIRT_PCIE] = 3, /* ... to 6 */
  [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
  };
  
 @@ -312,7 +315,7 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
  }
  }
  
 -static void fdt_add_gic_node(const VirtBoardInfo *vbi)
 +static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
  {
  uint32_t gic_phandle;
  
 @@ -331,9 +334,11 @@ static void fdt_add_gic_node(const VirtBoardInfo *vbi)
   2, vbi-memmap[VIRT_GIC_CPU].base,
   2, vbi-memmap[VIRT_GIC_CPU].size);
  qemu_fdt_setprop_cell(vbi-fdt, /intc, phandle, gic_phandle);
 +
 +return gic_phandle;
  }
  
 -static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
 +static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
  {
  /* We create a standalone GIC v2 */
  DeviceState *gicdev;
 @@ -380,7 +385,7 @@ static void create_gic(const VirtBoardInfo *vbi, 
 qemu_irq *pic)
  pic[i] = qdev_get_gpio_in(gicdev, i);
  }
  
 -fdt_add_gic_node(vbi);
 +return fdt_add_gic_node(vbi);
  }
  
  static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
 @@ -556,6 +561,100 @@ static void create_fw_cfg(const VirtBoardInfo *vbi)
  g_free(nodename);
  }
  
 +static void create_pcie_irq_map(const VirtBoardInfo *vbi, uint32_t 
 gic_phandle,
 +int first_irq, const char *nodename)
 +{
 +int devfn, pin;
 +uint32_t full_irq_map[4 * 4 * 8] = { 0 };
 +uint32_t *irq_map = full_irq_map;
 +
 +for (devfn = 0; devfn = 0x18; devfn+=0x8) {

 devfn += 0x8 (spaces)
 
 Yeah, I had it like that and it looked uglier. I guess it's a matter of
 personal preference?

You don't have coding standards for this ?

 

 +for (pin = 0; pin  4; pin++) {
 +int irq_type = GIC_FDT_IRQ_TYPE_SPI;
 +int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % 
 PCI_NUM_PINS);
 +int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
 +int i;
 +
 +uint32_t map[] = {
 +devfn  8, 0, 0,   /* devfn */
 +pin + 1,/* PCI pin */
 +gic_phandle, irq_type, irq_nr, irq_level }; /* GIC irq */
 +
 +/* Convert map to big endian */
 +for 

Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE

2015-01-27 Thread Markus Armbruster
Eric Blake ebl...@redhat.com writes:

 On 01/26/2015 01:48 AM, Javier Celaya wrote:
 Sorry, I forgot to patch the command-line help. Hope it helps.
 


 Recently, SPICE included the lz4 compression algorithm. This patch adds
 a command line option to select it.


 How is libvirt going to introspect whether the command line supports
 this option?  Is there some QMP command that lists the set of valid
 compression formats understood by a given qemu binary?

 No, patching the command line --help does NOT help libvirt.  It needs to
 be discoverable via QMP to be introspectible, as scraping --help output
 is not machine-friendly.  (That said, you DO want to expose it in --help
 output; I'm just complaining that --help output alone is not enough).

We should really, really, really provide access to (the relevant subset
of) the QAPI schema over QMP!  Until we get that, useful progress is
delayed by problems like this one, and we keep growing special-purpose
solutions to problems like this one.



Re: [Qemu-devel] [PATCH 0/4] nbd: iotest fixes and error message improvement

2015-01-27 Thread Paolo Bonzini


On 27/01/2015 03:02, Max Reitz wrote:
 This series is a follow-up to my previous patch iotests: Specify format
 for qemu-nbd and as such relies on it.
 
 The first three patches of this series fix the qemu-iotests so they once
 again pass when using NBD.
 
 The fourth patch of this series improves NBD's error message for
 establishing connections, especially if the server's and the client's
 NBD version differs (which, until now, was simply Bad magic received).

Good idea!

 Max Reitz (4):
   iotests: Fix 083
   iotests: Fix 100 for nbd
   iotests: Fix 104 for NBD
   nbd: Improve error messages
 
  block/nbd-client.c   |  4 +-
  block/nbd-client.h   |  2 +-
  block/nbd.c  |  2 +-
  include/block/nbd.h  |  2 +-
  nbd.c| 42 -
  qemu-nbd.c   |  7 +++-
  tests/qemu-iotests/083   |  3 +-
  tests/qemu-iotests/083.out   | 81 
 ++--
  tests/qemu-iotests/100   | 12 ++
  tests/qemu-iotests/104   |  9 ++---
  tests/qemu-iotests/common.filter |  1 +
  11 files changed, 82 insertions(+), 83 deletions(-)
 

Reviewed-by: Paolo Bonzini pbonz...@redhat.com



Re: [Qemu-devel] [PATCH v2 3/4] arm: Add PCIe host bridge in virt machine

2015-01-27 Thread Peter Maydell
On 27 January 2015 at 09:24, Claudio Fontana claudio.font...@huawei.com wrote:
 On 22.01.2015 16:52, Alexander Graf wrote:
 On 22.01.15 16:28, Claudio Fontana wrote:
 Alex wrote;
 +for (devfn = 0; devfn = 0x18; devfn+=0x8) {

 devfn += 0x8 (spaces)

 Yeah, I had it like that and it looked uglier. I guess it's a matter of
 personal preference?

 You don't have coding standards for this ?

Not formal ones, but (a) I think += should have spaces aronud
it and (b) I'm a bit surprised if checkpatch doesn't catch
this (it certainly attempts to...)

-- PMM



Re: [Qemu-devel] [PATCH 2/2] bootdevice: update boot_order in MachineState

2015-01-27 Thread Gonglei
On 2015/1/27 16:57, Dinar Valeev wrote:

 On 01/27/2015 03:51 AM, Gonglei wrote:
 On 2015/1/27 7:52, dval...@suse.de wrote:

 From: Dinar Valeev dval...@suse.com

 on sPAPR we need to update boot_order in MachineState in case it
 got changed on reset.

 Signed-off-by: Dinar Valeev dval...@suse.com
 ---
  bootdevice.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/bootdevice.c b/bootdevice.c
 index 5914417..4f11a06 100644
 --- a/bootdevice.c
 +++ b/bootdevice.c
 @@ -26,6 +26,7 @@
  #include qapi/visitor.h
  #include qemu/error-report.h
  #include hw/hw.h
 +#include hw/boards.h
  
  typedef struct FWBootEntry FWBootEntry;
  
 @@ -50,6 +51,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, 
 void *opaque)
  void qemu_boot_set(const char *boot_order, Error **errp)
  {
  Error *local_err = NULL;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +machine-boot_order = boot_order;
  
  if (!boot_set_handler) {
  error_setg(errp, no function defined to set boot device list for

 Have you registered boot set handler on ppc/sPAPR platform by calling
 qemu_register_boot_set()? Otherwise qemu_boot_set function
  will return error.
 No, I set boot_order on each machine reset. My tests are showing it works 
 without an error.

That's interesting. Does this function be called?
Would you debug it by setting a breakpoint ?

Regards,
-Gonglei

 Previous patch was using qemu_register_boot_set, but Alexender Graf, 
 suggested me to use MachineState and simply update on each guest reset.
 

 Regards,
 -Gonglei

 






Re: [Qemu-devel] [PATCH v4 37/47] Page request: Consume pages off the post-copy queue

2015-01-27 Thread Dr. David Alan Gilbert
* David Gibson (da...@gibson.dropbear.id.au) wrote:
 On Wed, Jan 14, 2015 at 08:13:27PM +, Dr. David Alan Gilbert wrote:
  * David Gibson (da...@gibson.dropbear.id.au) wrote:
   On Fri, Oct 03, 2014 at 06:47:43PM +0100, Dr. David Alan Gilbert (git) 
   wrote:
From: Dr. David Alan Gilbert dgilb...@redhat.com

When transmitting RAM pages, consume pages that have been queued by
MIG_RPCOMM_REQPAGE commands and send them ahead of normal page scanning.

Note:
  a) After a queued page the linear walk carries on from after the
unqueued page; there is a reasonable chance that the destination
was about to ask for other closeby pages anyway.

  b) We have to be careful of any assumptions that the page walking
code makes, in particular it does some short cuts on its first linear
walk that break as soon as we do a queued page.

Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
 arch_init.c | 149 
++--
 1 file changed, 125 insertions(+), 24 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 72f9e17..a945990 100644
+
+/*
+ * Don't break host-page chunks up with queue items
   
   Why does this matter?
  
  See the comment you make in a few patches time, it's about being able
  to place the pages atomically on the destination.
 
 Hmm.  But if the destination has to wait for all the pieces of a host
 page to arrive anyway, does it really make any difference if they're
 contiguous in the stream?

The problem is knowing where to put the arriving target-pages until you've
got a full host-page; you've got to put the arriving TP into a temporary
until you have the full set, if they're not contiguous in the stream
then you have to have multiple temporarys dealing with the set of outstanding
host pages that you've not got the full set for; and you've still got to be
careful on the sending side to have a bounded-number of host-pages on the run
at any time.   Making that bound 1 makes the code simpler.

+ * so only unqueue if,
+ *   a) The last item came from the queue anyway
+ *   b) The last sent item was the last target-page in a host 
page
+ */
+if (last_was_from_queue || (!last_sent_block) ||
+((last_offset  (hps - 1)) == (hps - TARGET_PAGE_SIZE))) {
+tmpblock = ram_save_unqueue_page(ms, tmpoffset, 
bitoffset);
 }
-if (offset = block-length) {
-offset = 0;
-block = QTAILQ_NEXT(block, next);
-if (!block) {
-block = QTAILQ_FIRST(ram_list.blocks);
-complete_round = true;
-ram_bulk_stage = false;
+
+if (tmpblock) {
+/* We've got a block from the postcopy queue */
+DPRINTF(%s: Got postcopy item '%s' offset=%zx 
bitoffset=%zx,
+__func__, tmpblock-idstr, tmpoffset, bitoffset);
+/* We're sending this page, and since it's postcopy 
nothing else
+ * will dirty it, and we must make sure it doesn't get 
sent again.
+ */
+if (!migration_bitmap_clear_dirty(bitoffset  
TARGET_PAGE_BITS)) {
   
   Ugh.. that's kind of subtle.  I think it would be clearer if you work
   in terms of a ram_addr_t throughout, rather than bitoffset whose
   meaning is not terribly obvious.
  
  I've changed it to ram_addr_t as requested; it's slightly clearer but there
  are a few places where we're dealing with the sentmap where we now need to 
  shift
  the other way.  In the end ram_addr_t is really a scaled offset into those
  bitmaps.
 
 Right, but to someone who isn't deeply familiar with the code, they're
 more likely to understand what the ram address means than the bitmap
 offset.

Fair enough.

Dave

 
 -- 
 David Gibson  | I'll have my music baroque, and my code
 david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
 _other_
   | _way_ _around_!
 http://www.ozlabs.org/~dgibson


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



Re: [Qemu-devel] [PATCH 2/2] bootdevice: update boot_order in MachineState

2015-01-27 Thread Dinar Valeev
On 01/27/2015 10:18 AM, Gonglei wrote:
 On 2015/1/27 16:57, Dinar Valeev wrote:
 
 On 01/27/2015 03:51 AM, Gonglei wrote:
 On 2015/1/27 7:52, dval...@suse.de wrote:

 From: Dinar Valeev dval...@suse.com

 on sPAPR we need to update boot_order in MachineState in case it
 got changed on reset.

 Signed-off-by: Dinar Valeev dval...@suse.com
 ---
  bootdevice.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/bootdevice.c b/bootdevice.c
 index 5914417..4f11a06 100644
 --- a/bootdevice.c
 +++ b/bootdevice.c
 @@ -26,6 +26,7 @@
  #include qapi/visitor.h
  #include qemu/error-report.h
  #include hw/hw.h
 +#include hw/boards.h
  
  typedef struct FWBootEntry FWBootEntry;
  
 @@ -50,6 +51,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, 
 void *opaque)
  void qemu_boot_set(const char *boot_order, Error **errp)
  {
  Error *local_err = NULL;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +machine-boot_order = boot_order;
  
  if (!boot_set_handler) {
  error_setg(errp, no function defined to set boot device list for

 Have you registered boot set handler on ppc/sPAPR platform by calling
 qemu_register_boot_set()? Otherwise qemu_boot_set function
  will return error.
 No, I set boot_order on each machine reset. My tests are showing it works 
 without an error.
 
 That's interesting. Does this function be called?
I'll see what I can do here. I recall once I debugged once option, I haven't 
seen
error message either.

 Would you debug it by setting a breakpoint ?
 
 Regards,
 -Gonglei
 
 Previous patch was using qemu_register_boot_set, but Alexender Graf, 
 suggested me to use MachineState and simply update on each guest reset.


 Regards,
 -Gonglei


 
 
 




[Qemu-devel] [PATCH v4] sheepdog: selectable object size support

2015-01-27 Thread Teruaki Ishizaki
Previously, qemu block driver of sheepdog used hard-coded VDI object size.
This patch enables users to handle block_size_shift value for
calculating VDI object size.

When you start qemu, you don't need to specify additional command option.

But when you create the VDI which doesn't have default object size
with qemu-img command, you specify block_size_shift option.

If you want to create a VDI of 8MB(1  23) object size,
you need to specify following command option.

 # qemu-img create -o block_size_shift=23 sheepdog:test1 100M

In addition, when you don't specify qemu-img command option,
a default value of sheepdog cluster is used for creating VDI.

 # qemu-img create sheepdog:test2 100M

Signed-off-by: Teruaki Ishizaki ishizaki.teru...@lab.ntt.co.jp
---
V4:
 - Limit a read/write buffer size for creating a preallocated VDI.
 - Replace a parse function for the block_size_shift option.
 - Fix an error message.

V3:
 - Delete the needless operation of buffer.
 - Delete the needless operations of request header.
   for SD_OP_GET_CLUSTER_DEFAULT.
 - Fix coding style problems.

V2:
 - Fix coding style problem (white space).
 - Add members, store_policy and block_size_shift to struct SheepdogVdiReq.
 - Initialize request header to use block_size_shift specified by user.
---
 block/sheepdog.c  |  138 ++---
 include/block/block_int.h |1 +
 2 files changed, 119 insertions(+), 20 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index be3176f..a43b947 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -37,6 +37,7 @@
 #define SD_OP_READ_VDIS  0x15
 #define SD_OP_FLUSH_VDI  0x16
 #define SD_OP_DEL_VDI0x17
+#define SD_OP_GET_CLUSTER_DEFAULT   0x18
 
 #define SD_FLAG_CMD_WRITE0x01
 #define SD_FLAG_CMD_COW  0x02
@@ -167,7 +168,8 @@ typedef struct SheepdogVdiReq {
 uint32_t base_vdi_id;
 uint8_t copies;
 uint8_t copy_policy;
-uint8_t reserved[2];
+uint8_t store_policy;
+uint8_t block_size_shift;
 uint32_t snapid;
 uint32_t type;
 uint32_t pad[2];
@@ -186,6 +188,21 @@ typedef struct SheepdogVdiRsp {
 uint32_t pad[5];
 } SheepdogVdiRsp;
 
+typedef struct SheepdogClusterRsp {
+uint8_t proto_ver;
+uint8_t opcode;
+uint16_t flags;
+uint32_t epoch;
+uint32_t id;
+uint32_t data_length;
+uint32_t result;
+uint8_t nr_copies;
+uint8_t copy_policy;
+uint8_t block_size_shift;
+uint8_t __pad1;
+uint32_t __pad2[6];
+} SheepdogClusterRsp;
+
 typedef struct SheepdogInode {
 char name[SD_MAX_VDI_LEN];
 char tag[SD_MAX_VDI_TAG_LEN];
@@ -1544,6 +1561,7 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t 
*vdi_id, int snapshot,
 hdr.vdi_size = s-inode.vdi_size;
 hdr.copy_policy = s-inode.copy_policy;
 hdr.copies = s-inode.nr_copies;
+hdr.block_size_shift = s-inode.block_size_shift;
 
 ret = do_req(fd, s-aio_context, (SheepdogReq *)hdr, buf, wlen, rlen);
 
@@ -1569,9 +1587,12 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t 
*vdi_id, int snapshot,
 static int sd_prealloc(const char *filename, Error **errp)
 {
 BlockDriverState *bs = NULL;
+BDRVSheepdogState *base = NULL;
+unsigned long buf_size;
 uint32_t idx, max_idx;
+uint32_t object_size;
 int64_t vdi_size;
-void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
+void *buf = NULL;
 int ret;
 
 ret = bdrv_open(bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
@@ -1585,18 +1606,24 @@ static int sd_prealloc(const char *filename, Error 
**errp)
 ret = vdi_size;
 goto out;
 }
-max_idx = DIV_ROUND_UP(vdi_size, SD_DATA_OBJ_SIZE);
+
+base = bs-opaque;
+object_size = (UINT32_C(1)  base-inode.block_size_shift);
+buf_size = MIN(object_size, SD_DATA_OBJ_SIZE);
+buf = g_malloc0(buf_size);
+
+max_idx = DIV_ROUND_UP(vdi_size, buf_size);
 
 for (idx = 0; idx  max_idx; idx++) {
 /*
  * The created image can be a cloned image, so we need to read
  * a data from the source image.
  */
-ret = bdrv_pread(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
+ret = bdrv_pread(bs, idx * buf_size, buf, buf_size);
 if (ret  0) {
 goto out;
 }
-ret = bdrv_pwrite(bs, idx * SD_DATA_OBJ_SIZE, buf, SD_DATA_OBJ_SIZE);
+ret = bdrv_pwrite(bs, idx * buf_size, buf, buf_size);
 if (ret  0) {
 goto out;
 }
@@ -1669,6 +1696,21 @@ static int parse_redundancy(BDRVSheepdogState *s, const 
char *opt)
 return 0;
 }
 
+static int parse_block_size_shift(BDRVSheepdogState *s, QemuOpts *opt)
+{
+struct SheepdogInode *inode = s-inode;
+inode-block_size_shift =
+(uint8_t)qemu_opt_get_number_del(opt, block_size_shift, 0);
+if (inode-block_size_shift == 0) {
+/* block_size_shift is set for cluster default value by sheepdog */
+return 0;
+} else if (inode-block_size_shift  20 || 

[Qemu-devel] [PATCH] scsi: Fix scsi_req_cancel_async for no aiocb req

2015-01-27 Thread Fam Zheng
scsi_req_cancel_complete is responsible for releasing the request, so we
shouldn't skip it in any case. This doesn't affect the only existing
caller, virtio-scsi, but is useful for other devices once they use it.

Suggested-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
---
 hw/scsi/scsi-bus.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 9b740a3..db39ae0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1756,6 +1756,8 @@ void scsi_req_cancel_async(SCSIRequest *req, Notifier 
*notifier)
 req-io_canceled = true;
 if (req-aiocb) {
 blk_aio_cancel_async(req-aiocb);
+} else {
+scsi_req_cancel_complete(req);
 }
 }
 
-- 
1.9.3




Re: [Qemu-devel] [PATCH v2 3/3] pc-dimm: Add Error argument to pc_existing_dimms_capacity

2015-01-27 Thread Igor Mammedov
On Tue, 27 Jan 2015 09:35:02 +0530
Bharata B Rao bhar...@linux.vnet.ibm.com wrote:

 Now that pc_existing_dimms_capacity() is an API, include Error pointer
 as an argument and modify the caller appropriately.
 
 Suggested-by: Igor Mammedov imamm...@redhat.com
 Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
 ---
  hw/i386/pc.c |  4 ++--
  hw/mem/pc-dimm.c | 32 ++--
  include/hw/mem/pc-dimm.h |  2 +-
  3 files changed, 25 insertions(+), 13 deletions(-)
 
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index 2ec45a4..c7af6aa 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -1584,8 +1584,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
  goto out;
  }
  
 -if (pc_existing_dimms_capacity(OBJECT(machine), 
 existing_dimms_capacity)) {
 -error_setg(local_err, failed to get total size of existing DIMMs);
 +existing_dimms_capacity = pc_existing_dimms_capacity(local_err);
 +if (local_err) {
  goto out;
  }
  
 diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
 index f02ce6e..18cdc54 100644
 --- a/hw/mem/pc-dimm.c
 +++ b/hw/mem/pc-dimm.c
 @@ -22,32 +22,44 @@
  #include qemu/config-file.h
  #include qapi/visitor.h
  #include qemu/range.h
 -#include qapi/qmp/qerror.h
  
 -int pc_existing_dimms_capacity(Object *obj, void *opaque)
 +typedef struct pc_dimms_capacity {
 + uint64_t size;
 + Error**errp;
 +} pc_dimms_capacity;
 +
 +static int pc_existing_dimms_capacity_internal(Object *obj, void *opaque)
  {
 -Error *local_err = NULL;
 -uint64_t *size = opaque;
 +pc_dimms_capacity *cap = opaque;
 +uint64_t *size = cap-size;
  
  if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
  DeviceState *dev = DEVICE(obj);
  
  if (dev-realized) {
  (*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
 -local_err);
 +cap-errp);
  }
  
 -if (local_err) {
 -qerror_report_err(local_err);
 -error_free(local_err);
 +if (cap-errp  *cap-errp) {
  return 1;
  }
  }
 -
 -object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
 +object_child_foreach(obj, pc_existing_dimms_capacity_internal, opaque);
  return 0;
  }
  
 +uint64_t pc_existing_dimms_capacity(Error **errp)
 +{
 +pc_dimms_capacity cap;
 +
 +cap.size = 0;
 +cap.errp = errp;
 +
 +pc_existing_dimms_capacity_internal(qdev_get_machine(), cap);
 +return cap.size;
 +}
 +
  int qmp_pc_dimm_device_list(Object *obj, void *opaque)
  {
  MemoryDeviceInfoList ***prev = opaque;
 diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
 index bbfa53f..f7b80b4 100644
 --- a/include/hw/mem/pc-dimm.h
 +++ b/include/hw/mem/pc-dimm.h
 @@ -78,5 +78,5 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
  int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
  
  int qmp_pc_dimm_device_list(Object *obj, void *opaque);
 -int pc_existing_dimms_capacity(Object *obj, void *opaque);
 +uint64_t pc_existing_dimms_capacity(Error **errp);
  #endif

Reviewed-by: Igor Mammedov imamm...@redhat.com



Re: [Qemu-devel] [RFC PATCH 03/11] hw/arm/virt-acpi-build: Generate RSDP table

2015-01-27 Thread Shannon Zhao
On 2015/1/26 18:22, Igor Mammedov wrote:
 On Sat, 24 Jan 2015 17:21:12 +0800
 Shannon Zhao zhaoshengl...@huawei.com wrote:
 
  RSDP points to XSDT which in turn points to other tables.
  
  Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com
  ---
   hw/arm/virt-acpi-build.c |   22 ++
   1 files changed, 22 insertions(+), 0 deletions(-)
  
  diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
  index 4eed0a3..9c3971a 100644
  --- a/hw/arm/virt-acpi-build.c
  +++ b/hw/arm/virt-acpi-build.c
  @@ -86,6 +86,28 @@ static inline void acpi_add_table(GArray 
  *table_offsets, GArray *table_data)
   static GArray *
   build_rsdp(GArray *rsdp_table, GArray *linker, uint64_t xsdt)
   {
  +AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
  +
  +bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 16,
  + true /* fseg memory */);
  +
  +memcpy(rsdp-signature, RSD PTR , sizeof(rsdp-signature));
  +memcpy(rsdp-oem_id, ACPI_VIRT_QEMU_STR_6, sizeof(rsdp-oem_id));
  +rsdp-length = cpu_to_le32(sizeof(*rsdp));
  +rsdp-revision = 0x02;
  +
  +/* Point to XSDT */
  +rsdp-xsdt_physical_address = cpu_to_le64(xsdt);
 RSDP should be created after XSDT so XSDT pointer would be correct,
 perhaps it's wrong patch ordering
 
Hi,

About this I think the patch order is not wrong because at the moment we don't 
enable ACPI.
So this code shouldn't execute. When all tables are generated OK, we could 
enable CONFIG_ACPI.


Thanks,
Shannon




Re: [Qemu-devel] [RFC PATCH 03/11] hw/arm/virt-acpi-build: Generate RSDP table

2015-01-27 Thread Igor Mammedov
On Tue, 27 Jan 2015 17:36:29 +0800
Shannon Zhao zhaoshengl...@huawei.com wrote:

 On 2015/1/26 18:22, Igor Mammedov wrote:
  On Sat, 24 Jan 2015 17:21:12 +0800
  Shannon Zhao zhaoshengl...@huawei.com wrote:
  
   RSDP points to XSDT which in turn points to other tables.
   
   Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com
   ---
hw/arm/virt-acpi-build.c |   22 ++
1 files changed, 22 insertions(+), 0 deletions(-)
   
   diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
   index 4eed0a3..9c3971a 100644
   --- a/hw/arm/virt-acpi-build.c
   +++ b/hw/arm/virt-acpi-build.c
   @@ -86,6 +86,28 @@ static inline void acpi_add_table(GArray 
   *table_offsets, GArray *table_data)
static GArray *
build_rsdp(GArray *rsdp_table, GArray *linker, uint64_t xsdt)
{
   +AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
   +
   +bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, 16,
   + true /* fseg memory */);
   +
   +memcpy(rsdp-signature, RSD PTR , sizeof(rsdp-signature));
   +memcpy(rsdp-oem_id, ACPI_VIRT_QEMU_STR_6, sizeof(rsdp-oem_id));
   +rsdp-length = cpu_to_le32(sizeof(*rsdp));
   +rsdp-revision = 0x02;
   +
   +/* Point to XSDT */
   +rsdp-xsdt_physical_address = cpu_to_le64(xsdt);
  RSDP should be created after XSDT so XSDT pointer would be correct,
  perhaps it's wrong patch ordering
  
 Hi,
 
 About this I think the patch order is not wrong because at the moment we 
 don't enable ACPI.
Yes would work, but it still more clear when tables are created in order
in which they are used not backwards.

 So this code shouldn't execute. When all tables are generated OK, we could 
 enable CONFIG_ACPI.
 
 
 Thanks,
 Shannon
 
 




Re: [Qemu-devel] [PATCH v4 43/47] Host page!=target page: Cleanup bitmaps

2015-01-27 Thread Dr. David Alan Gilbert
* David Gibson (da...@gibson.dropbear.id.au) wrote:
 On Wed, Dec 17, 2014 at 06:21:34PM +, Dr. David Alan Gilbert wrote:
  * David Gibson (da...@gibson.dropbear.id.au) wrote:
   On Fri, Oct 03, 2014 at 06:47:49PM +0100, Dr. David Alan Gilbert (git) 
   wrote:
From: Dr. David Alan Gilbert dgilb...@redhat.com

Prior to the start of postcopy, ensure that everything that will
be transferred later is a whole host-page in size.

This is accomplished by discarding partially transferred host pages
and marking any that are partially dirty as fully dirty.

Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
 arch_init.c | 112 
+++-
 1 file changed, 111 insertions(+), 1 deletion(-)

diff --git a/arch_init.c b/arch_init.c
index 1fe4fab..aac250c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1024,7 +1024,6 @@ static uint32_t get_32bits_map(unsigned long 
*map, int64_t start)
  * A helper to put 32 bits into a bit map; trivial for 
HOST_LONG_BITS=32
  * messier for 64; the bitmaps are actually long's that are 32 or 64bit
  */
-__attribute__ (( unused )) /* Until later in patch series */
 static void put_32bits_map(unsigned long *map, int64_t start,
uint32_t v)
 {
@@ -1153,15 +1152,126 @@ static int pc_each_ram_discard(MigrationState 
*ms)
 }
 
 /*
+ * Utility for the outgoing postcopy code.
+ *
+ * Discard any partially sent host-page size chunks, mark any partially
+ * dirty host-page size chunks as all dirty.
+ *
+ * Returns: 0 on success
+ */
+static int postcopy_chunk_hostpages(MigrationState *ms)
+{
+struct RAMBlock *block;
+unsigned int host_bits = sysconf(_SC_PAGESIZE) / TARGET_PAGE_SIZE;
+uint32_t host_mask;
+
+/* Should be a power of 2 */
+assert(host_bits  !(host_bits  (host_bits - 1)));
+/*
+ * If the host_bits isn't a division of 32 (the minimum long size)
+ * then the code gets a lot more complex; disallow for now
+ * (I'm not aware of a system where it's true anyway)
+ */
+assert((32 % host_bits) == 0);
   
   This assert makes the first one redundant.
  
  True I guess, removed the power of 2 check.
  
  snip
  
+/*
  * Transmit the set of pages to be discarded after precopy to the 
target
  * these are pages that have been sent previously but have been dirtied
  * Hopefully this is pretty sparse
  */
 int ram_postcopy_send_discard_bitmap(MigrationState *ms)
 {
+int ret;
+
 /* This should be our last sync, the src is now paused */
 migration_bitmap_sync();
 
+/* Deal with TPS != HPS */
+ret = postcopy_chunk_hostpages(ms);
+if (ret) {
+return ret;
+}
   
   This really seems like a bogus thing to be doing on the outgoing
   migration side.  Doesn't the host page size constraint come from the
   destination (due to the need to atomically instate pages).  Source
   host page size == destination host page size doesn't seem like it
   should be an inherent constraint
  
  It's not an inherent constraint; it just makes life messier. I had
  some code to deal with it but it complicates things even more, and
  I've not got anything to test that rare case with; if someone is
  desperate for it then it can be added.
 
 So, I'm all for deferring implementation improvements that we don't
 need for the time being.
 
 What worries me though, is having the source have to make assumptions
 about how the migration stream will be processed on the destination
 that aren't somehow baked into the protocol itself.  i.e. I think we
 should really try to avoid the possibility of migration streams that
 are structurally sound, and look like they should be valid, but
 aren't, because of subtle constraints in the order and manner in which
 the destination needs to process the individual chunks.

Agreed; see below.

   and it's not clear why you can't do
   this rounding out to host page sized chunks on the receive end.
  
  The source keeps track of which pages still need sending, and so
  has to update that list when it tells the destination to perform
  a discard.
 
 Ah.
 
  If the destination discards more than the source told it to (for
  example because it has bigger host-pages) the source would need
  to update it's map of the pages that still need sending.
 
 I'm beginning to wonder if what we really need is for early in the
 migration process the destination to tell the host what granularity of
 updates it can handle (based on its page size).
 
 Perhaps the short summary is that I don't think we need to actually
 handle the case of different source and dest host page sizes.  BUT,
 if that does happen the migration process should be able to detect
 that that's what's 

[Qemu-devel] [PATCH RESEND 47/50] blockdev: Add read-only option to blockdev-change-medium

2015-01-27 Thread Max Reitz
Add an option to qmp_blockdev_change_medium() which allows changing the
read-only status of the block device whose medium is changed.

Some drives do not have a inherently fixed read-only status; for
instance, floppy disks can be set read-only or writable independently of
the drive. Some users may find it useful to be able to therefore change
the read-only status of a block device when changing the medium.

Signed-off-by: Max Reitz mre...@redhat.com
---
 blockdev.c   | 25 -
 hmp.c|  2 +-
 qapi/block-core.json | 24 +++-
 qmp-commands.hx  | 24 +++-
 qmp.c|  3 ++-
 5 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 2ada2b1..8ed2fec 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2014,6 +2014,8 @@ void qmp_blockdev_insert_medium(const char *device, const 
char *node_name,
 
 void qmp_blockdev_change_medium(const char *device, const char *filename,
 bool has_format, const char *format,
+bool has_read_only,
+BlockdevChangeReadOnlyMode read_only,
 Error **errp)
 {
 BlockBackend *blk;
@@ -2034,7 +2036,28 @@ void qmp_blockdev_change_medium(const char *device, 
const char *filename,
 }
 
 blk_rs = blk_get_root_state(blk);
-bdrv_flags = blk_rs-read_only ? 0 : BDRV_O_RDWR;
+
+if (!has_read_only) {
+read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;
+}
+
+switch (read_only) {
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN:
+bdrv_flags = blk_rs-read_only ? 0 : BDRV_O_RDWR;
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RO:
+bdrv_flags = 0;
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RW:
+bdrv_flags = BDRV_O_RDWR;
+break;
+
+default:
+abort();
+}
+
 bdrv_flags |= blk_rs-open_flags  ~BDRV_O_RDWR;
 
 if (has_format) {
diff --git a/hmp.c b/hmp.c
index 300e7d8..dbf0947 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1190,7 +1190,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 }
 qmp_change(vnc, target, !!arg, arg, err);
 } else {
-qmp_blockdev_change_medium(device, target, !!arg, arg, err);
+qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, err);
 if (err 
 error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
 error_free(err);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index d3c3ca7..eb2724e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1785,6 +1785,24 @@
 
 
 ##
+# @BlockdevChangeReadOnlyMode:
+#
+# Specifies the new read-only mode of a block device subject to the
+# @blockdev-change-medium command.
+#
+# @retain:  Retains the current read-only mode
+#
+# @ro:  Makes the device read-only
+#
+# @rw:  Makes the device writable
+#
+# Since: 2.3
+##
+{ 'enum': 'BlockdevChangeReadOnlyMode',
+  'data': ['retain', 'ro', 'rw'] }
+
+
+##
 # @blockdev-change-medium:
 #
 # Changes the medium inserted into a block device by ejecting the current 
medium
@@ -1799,12 +1817,16 @@
 # @format:  #optional, format to open the new image with (defaults to the
 #   probed format)
 #
+# @read-only:   #optional, change the read-only mode of the device; defaults to
+#   'retain'
+#
 # Since: 2.3
 ##
 { 'command': 'blockdev-change-medium',
   'data': { 'device': 'str',
 'filename': 'str',
-'*format': 'str' } }
+'*format': 'str',
+'*read-only': 'BlockdevChangeReadOnlyMode' } }
 
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1987a09..f14953a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3855,7 +3855,7 @@ EQMP
 
 {
 .name   = blockdev-change-medium,
-.args_type  = device:B,filename:F,format:s?,
+.args_type  = device:B,filename:F,format:s?,read-only:s?,
 .mhandler.cmd_new = qmp_marshal_input_blockdev_change_medium,
 },
 
@@ -3871,6 +3871,8 @@ Arguments:
 - device: device name (json-string)
 - filename: filename of the new image (json-string)
 - format: format of the new image (json-string, optional)
+- read-only: new read-only mode (json-string, optional)
+  - Possible values: retain (default), ro, rw
 
 Examples:
 
@@ -3882,6 +3884,26 @@ Examples:
 format: raw } }
 - { return: {} }
 
+2. Load a read-only medium into a writable drive
+
+- { execute: blockdev-change-medium,
+ arguments: { device: isa-fd0,
+filename: /srv/images/ro.img,
+format: raw,
+read-only: retain } }
+
+- { error:
+ { class: GenericError,
+   desc: Could not open '/srv/images/ro.img': Permission denied } }
+
+- { execute: blockdev-change-medium,
+ arguments: { device: isa-fd0,
+ 

  1   2   3   4   >