Re: [Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files

[Markus Armbruster]

Eduardo Habkost  writes:

> With this, code that uses symbols from qemu-file.c don't need to bring extra
> dependencies because of the actual QEMUFile operation implementations.

Each case of omitting the "extra dependencies" should be visible in
makefiles as "prerequisites include qemu-file.o, but not all
qemu-file-*.o".  I can see one: tests/Makefile has just
qemu-file-unix.o, but not qemu-file-stdio.o.  Out of curiosity: will
there be more?

Regardless:

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v3 0/6] Q35: Implement -cdrom/-hda sugar

[Markus Armbruster]

John Snow  writes:

> The Q35 board initialization does not currently bother to look
> for any drives added by the various syntactical sugar shorthands
> to be added to the AHCI HBA. These include -hda through -hdd,
> -cdrom, and -drive if=ide shorthands.
>
> An obstacle to having implemented this sooner is debate over
> whether or not to add an additional interface type, and how to
> manage the different units-per-bus mappings of various HBA
> implementations.
>
> This patch series:
> (1) Does not add IF_AHCI, but reuses IF_IDE
> (2) Allows the if_max_devs table to be overridden
> (3) Adds this override to the Q35 board type.
> (4) Finally, adds implementation to Q35 initialization.

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH] qemu-file: Add copyright header to qemu-file.c

[Markus Armbruster]

"Dr. David Alan Gilbert"  writes:

> * Eduardo Habkost (ehabk...@redhat.com) wrote:
>> The person who created qemu-file.c (me, on commit
>> 093c455a8c6d8f715eabd8c8d346f08f17d686ec) didn't add a copyright/license
>> header to the file, even though the whole code was copied from savevm.c
>> (which had a copyright/license header).
>> 
>> To correct this, copy the copyright information and license from
>> savevm.c, that's where the original code came from.
>> 
>> Luckily, very few changes were made on qemu-file.c after it was created.
>> All the authors who touched the code are being CCed, so they can confirm
>> if they are OK with the copyright/license information being added.
>
> Since it's come from vl.c that seems to make sense.
> It should probably go to the stable branch as well, since reading the 
> ~/LICENSE
> file would make you think it's GPLd.
>
> Reviewed-by: Dr. David Alan Gilbert 

Since we released it that way, licensees could in good faith assume they
got it under GPL.

My contribution was just a one-liner bug fix.  Regardless:

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH] qemu-file: Add copyright header to qemu-file.c

[Alexey Kardashevskiy]

On 10/02/2014 03:47 AM, Eduardo Habkost wrote:
> The person who created qemu-file.c (me, on commit
> 093c455a8c6d8f715eabd8c8d346f08f17d686ec) didn't add a copyright/license
> header to the file, even though the whole code was copied from savevm.c
> (which had a copyright/license header).
> 
> To correct this, copy the copyright information and license from
> savevm.c, that's where the original code came from.
> 
> Luckily, very few changes were made on qemu-file.c after it was created.
> All the authors who touched the code are being CCed, so they can confirm
> if they are OK with the copyright/license information being added.
> 
> Cc: Dr. David Alan Gilbert 
> Cc: Alexey Kardashevskiy 
> Cc: Markus Armbruster 
> Cc: Juan Quintela 
> Signed-off-by: Eduardo Habkost 


Acked-by: Alexey Kardashevskiy 


> ---
>  qemu-file.c | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/qemu-file.c b/qemu-file.c
> index a8e3912..bd5d4af 100644
> --- a/qemu-file.c
> +++ b/qemu-file.c
> @@ -1,3 +1,26 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
>  #include "qemu-common.h"
>  #include "qemu/iov.h"
>  #include "qemu/sockets.h"
> 


-- 
Alexey

[Qemu-devel] [Bug 1376533] [NEW] Copyright year should be updated in vl.c

[Bruce Cran]

Public bug reported:

When specifying '--version', qemu prints the version along with
'Copyright (c) 2003-2008'.

Some users may think that it hasn't been updated since 2008, so the end
year in version() in vl.c should probably be updated around the start of
each new year.

Found in the qemu-2.1.2 source tarball.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Copyright year should be updated in vl.c

Status in QEMU:
  New

Bug description:
  When specifying '--version', qemu prints the version along with
  'Copyright (c) 2003-2008'.

  Some users may think that it hasn't been updated since 2008, so the
  end year in version() in vl.c should probably be updated around the
  start of each new year.

  Found in the qemu-2.1.2 source tarball.

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

[Qemu-devel] [PATCH 4/6] ahci: unify sglist preparation

[John Snow]

The intent of this patch is to further unify the creation and
deletion of the sglist used for all AHCI transfers, including
emulated PIO, ATAPI R/W, and native DMA R/W.

By replacing ahci_start_transfer's call to ahci_populate_sglist
with ahci_dma_prepare_buf, we reduce the number of direct calls
where we manipulate the scatter-gather list in the AHCI code.

To make this switch, the constant "0" passed as an offset
in ahci_dma_prepare_buf is adjusted to use io_buffer_offset.

For DMA pathways, this has no effect: io_buffer_offset is always
updated to 0 at the beginning of a DMA transfer loop regardless.
DMA pathways through ide_dma_cb() update the io_buffer_offset
accordingly, and for circumstances where we might make several
trips through this loop, this may actually correct a design flaw.

For PIO pathways, the newly updated ahci_dma_prepare_buf will
now prepare the sglist at the correct offset. It will also set
io_buffer_size, but this is not used in the cmd_read_pio or
cmd_write_pio pathways.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index db1d226..16cd248 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1091,7 +1091,7 @@ static void ahci_start_transfer(IDEDMA *dma)
 goto out;
 }
 
-if (!ahci_populate_sglist(ad, &s->sg, s->io_buffer_offset)) {
+if (ahci_dma_prepare_buf(dma, is_write)) {
 has_sglist = 1;
 }
 
@@ -1143,7 +1143,7 @@ static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write)
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
 IDEState *s = &ad->port.ifs[0];
 
-ahci_populate_sglist(ad, &s->sg, 0);
+ahci_populate_sglist(ad, &s->sg, s->io_buffer_offset);
 s->io_buffer_size = s->sg.size;
 
 DPRINTF(ad->port_no, "len=%#x\n", s->io_buffer_size);
-- 
1.9.3

[Qemu-devel] [PATCH 5/6] ide: Correct handling of malformed/short PRDTs

[John Snow]

This impacts both BMDMA and AHCI HBA interfaces for IDE.
Currently, we confuse the difference between a PRD having
"0 bytes" and a PRD having "0 complete sectors."

This leads to, in the BMDMA case, leaked memory for short PRDTs,
and infinite loops in the AHCI case.

the "prepare_buf" callback is reworked to return 0 if it could
not allocate a full sector's worth of buffer space, instead of
returning non-zero if it allocated any number of bytes.

ide_dma_cb adds a call to commit_buf in order to delete
the short PRDT that it will not attempt to use to finish
the DMA operation.

This patch corrects both occurrences and adds an assertion to
prevent future regression. This assertion is tested in the
existing ide-test, and is covered in a forthcoming AHCI test.

Signed-off-by: John Snow 
---
 dma-helpers.c | 3 +++
 hw/ide/ahci.c | 2 +-
 hw/ide/core.c | 1 +
 hw/ide/pci.c  | 5 +++--
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/dma-helpers.c b/dma-helpers.c
index 7f86e18..f51d6ee 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -38,6 +38,9 @@ int dma_memory_set(AddressSpace *as, dma_addr_t addr, uint8_t 
c, dma_addr_t len)
 void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint,
   AddressSpace *as)
 {
+/* If this is true, you're leaking memory. */
+assert(qsg->sg == NULL);
+
 qsg->sg = g_malloc(alloc_hint * sizeof(ScatterGatherEntry));
 qsg->nsg = 0;
 qsg->nalloc = alloc_hint;
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 16cd248..67c1e36 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1147,7 +1147,7 @@ static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write)
 s->io_buffer_size = s->sg.size;
 
 DPRINTF(ad->port_no, "len=%#x\n", s->io_buffer_size);
-return s->io_buffer_size != 0;
+return s->io_buffer_size / 512 != 0;
 }
 
 /**
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 7c1929e..82d01e8 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -732,6 +732,7 @@ void ide_dma_cb(void *opaque, int ret)
 /* The PRDs were too short. Reset the Active bit, but don't raise an
  * interrupt. */
 s->status = READY_STAT | SEEK_STAT;
+dma_buf_commit(s, false);
 goto eot;
 }
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 2397f35..3f643c2 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -74,8 +74,9 @@ static int bmdma_prepare_buf(IDEDMA *dma, int is_write)
 if (bm->cur_prd_len == 0) {
 /* end of table (with a fail safe of one page) */
 if (bm->cur_prd_last ||
-(bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE)
-return s->io_buffer_size != 0;
+(bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE) {
+return (s->io_buffer_size / 512) != 0;
+}
 pci_dma_read(pci_dev, bm->cur_addr, &prd, 8);
 bm->cur_addr += 8;
 prd.addr = le32_to_cpu(prd.addr);
-- 
1.9.3

[Qemu-devel] [PATCH 1/6] ahci: Correct PIO/D2H FIS responses

[John Snow]

Currently, the D2H FIS packets AHCI generates simply parrot back
the LBA that the guest sent to us in the cmd_fis. However, some
commands (like READ NATIVE MAX) modify the LBA registers as a
return value, through which the AHCI D2H FIS is the only response
mechanism. Thus, the D2H response should use the current register
values, not the initial ones.

This patch adjusts the LBA and drive select register responses for
PIO Setup and D2H FIS response packets.

Additionally, the PIO and D2H FIS responses copy too many bytes
from the command FIS that it is being generated from. Specifically,
byte 11 which is the Features(15:8) field for Register Host to
Device FIS packets, is instead reserved for the PIO Setup FIS and
should always be 0.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 48 +---
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 8978643..0529d68 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -599,6 +599,7 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
 uint8_t *pio_fis, *cmd_fis;
 uint64_t tbl_addr;
 dma_addr_t cmd_len = 0x80;
+IDEState *s = &ad->port.ifs[0];
 
 if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) {
 return;
@@ -628,21 +629,21 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t 
len)
 
 pio_fis[0] = 0x5f;
 pio_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
-pio_fis[2] = ad->port.ifs[0].status;
-pio_fis[3] = ad->port.ifs[0].error;
-
-pio_fis[4] = cmd_fis[4];
-pio_fis[5] = cmd_fis[5];
-pio_fis[6] = cmd_fis[6];
-pio_fis[7] = cmd_fis[7];
-pio_fis[8] = cmd_fis[8];
-pio_fis[9] = cmd_fis[9];
-pio_fis[10] = cmd_fis[10];
-pio_fis[11] = cmd_fis[11];
+pio_fis[2] = s->status;
+pio_fis[3] = s->error;
+
+pio_fis[4] = s->sector;
+pio_fis[5] = s->lcyl;
+pio_fis[6] = s->hcyl;
+pio_fis[7] = s->select;
+pio_fis[8] = s->hob_sector;
+pio_fis[9] = s->hob_lcyl;
+pio_fis[10] = s->hob_hcyl;
+pio_fis[11] = 0;
 pio_fis[12] = cmd_fis[12];
 pio_fis[13] = cmd_fis[13];
 pio_fis[14] = 0;
-pio_fis[15] = ad->port.ifs[0].status;
+pio_fis[15] = s->status;
 pio_fis[16] = len & 255;
 pio_fis[17] = len >> 8;
 pio_fis[18] = 0;
@@ -669,6 +670,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
 int i;
 dma_addr_t cmd_len = 0x80;
 int cmd_mapped = 0;
+IDEState *s = &ad->port.ifs[0];
 
 if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) {
 return;
@@ -686,17 +688,17 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
 
 d2h_fis[0] = 0x34;
 d2h_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
-d2h_fis[2] = ad->port.ifs[0].status;
-d2h_fis[3] = ad->port.ifs[0].error;
-
-d2h_fis[4] = cmd_fis[4];
-d2h_fis[5] = cmd_fis[5];
-d2h_fis[6] = cmd_fis[6];
-d2h_fis[7] = cmd_fis[7];
-d2h_fis[8] = cmd_fis[8];
-d2h_fis[9] = cmd_fis[9];
-d2h_fis[10] = cmd_fis[10];
-d2h_fis[11] = cmd_fis[11];
+d2h_fis[2] = s->status;
+d2h_fis[3] = s->error;
+
+d2h_fis[4] = s->sector;
+d2h_fis[5] = s->lcyl;
+d2h_fis[6] = s->hcyl;
+d2h_fis[7] = s->select;
+d2h_fis[8] = s->hob_sector;
+d2h_fis[9] = s->hob_lcyl;
+d2h_fis[10] = s->hob_hcyl;
+d2h_fis[11] = 0;
 d2h_fis[12] = cmd_fis[12];
 d2h_fis[13] = cmd_fis[13];
 for (i = 14; i < 20; i++) {
-- 
1.9.3

[Qemu-devel] [PATCH 6/6] ahci: Fix SDB FIS Construction

[John Snow]

The SDB FIS creation was mangled;
We were writing the error byte to byte 0,
and omitting the SDB FIS magic byte.

Though the SDB packet layout states that:
byte 0: Must be 0xA1 to indicate SDB FIS.
byte 1: Port multiplier select & other flags
byte 2: status byte.
byte 3: error byte.

This patch adds an SDB FIS structure with
human-readable names, and ensures that we
are filling the structure appropriately.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 18 +-
 hw/ide/ahci.h |  8 
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 67c1e36..8002e9e 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -569,24 +569,24 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, 
uint32_t finished)
 AHCIDevice *ad = &s->dev[port];
 AHCIPortRegs *pr = &ad->port_regs;
 IDEState *ide_state;
-uint8_t *sdb_fis;
+SDBFIS *sdb_fis;
 
 if (!s->dev[port].res_fis ||
 !(pr->cmd & PORT_CMD_FIS_RX)) {
 return;
 }
 
-sdb_fis = &ad->res_fis[RES_FIS_SDBFIS];
+sdb_fis = (SDBFIS *)&ad->res_fis[RES_FIS_SDBFIS];
 ide_state = &ad->port.ifs[0];
 
-/* clear memory */
-*(uint32_t*)sdb_fis = 0;
-
-/* write values */
-sdb_fis[0] = ide_state->error;
-sdb_fis[2] = ide_state->status & 0x77;
+sdb_fis->type = 0xA1;
+/* Interrupt pending & Notification bit */
+sdb_fis->flags = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
+sdb_fis->status = ide_state->status & 0x77;
+sdb_fis->error = ide_state->error;
+/* update SAct field in SDB_FIS */
 s->dev[port].finished |= finished;
-*(uint32_t*)(sdb_fis + 4) = cpu_to_le32(ad->finished);
+sdb_fis->payload = cpu_to_le32(ad->finished);
 
 /* Update shadow registers (except BSY 0x80 and DRQ 0x08) */
 pr->tfdata = (ad->port.ifs[0].error << 8) |
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 1543df7..a58dd09 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -327,6 +327,14 @@ typedef struct NCQFrame {
 uint8_t reserved10;
 } QEMU_PACKED NCQFrame;
 
+typedef struct SDBFIS {
+uint8_t type;
+uint8_t flags;
+uint8_t status;
+uint8_t error;
+uint32_t payload;
+} QEMU_PACKED SDBFIS;
+
 void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports);
 void ahci_uninit(AHCIState *s);
 
-- 
1.9.3

[Qemu-devel] [PATCH 2/6] ahci: Update byte count after DMA completion

[John Snow]

Currently, DMA read/write operations neglect to update
the byte count after a successful transfer like ATAPI
DMA read or PIO read/write operations do.

We correct this oversight by adding another callback into
the IDEDMAOps structure. The commit callback is called
whenever we are cleaning up a scatter-gather list.
AHCI can register this callback in order to update post-
transfer information such as byte count updates.

We use this callback in AHCI to consolidate where we delete
the SGlist as generated from the PRDT, as well as update the
byte count after the transfer is complete.

The QEMUSGList structure has an init flag added to it in order
to make qemu_sglist_destroy a nop if it is called when
there is no sglist, which simplifies cleanup and error paths.

This patch fixes several AHCI problems, notably Non-NCQ modes
of operation for Windows 7 as well as Hibernate support for Windows 7.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 41 +++--
 hw/ide/core.c | 11 +++
 hw/ide/internal.h |  2 ++
 3 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 0529d68..6966a94 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -48,6 +48,9 @@ static int handle_cmd(AHCIState *s,int port,int slot);
 static void ahci_reset_port(AHCIState *s, int port);
 static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis);
 static void ahci_init_d2h(AHCIDevice *ad);
+static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write);
+static void ahci_commit_buf(IDEDMA *dma, uint32_t tx_bytes);
+
 
 static uint32_t  ahci_port_read(AHCIState *s, int port, int offset)
 {
@@ -1104,16 +1107,12 @@ static void ahci_start_transfer(IDEDMA *dma)
 }
 }
 
-/* update number of transferred bytes */
-ad->cur_cmd->status = cpu_to_le32(le32_to_cpu(ad->cur_cmd->status) + size);
-
 out:
 /* declare that we processed everything */
 s->data_ptr = s->data_end;
 
-if (has_sglist) {
-qemu_sglist_destroy(&s->sg);
-}
+/* Update number of transferred bytes, destroy sglist */
+ahci_commit_buf(dma, size);
 
 s->end_transfer_func(s);
 
@@ -1134,6 +1133,11 @@ static void ahci_start_dma(IDEDMA *dma, IDEState *s,
 dma_cb(s, 0);
 }
 
+/**
+ * Called in DMA R/W chains to read the PRDT, utilizing ahci_populate_sglist.
+ * Not currently invoked by PIO R/W chains,
+ * which invoke ahci_populate_sglist via ahci_start_transfer.
+ */
 static int ahci_dma_prepare_buf(IDEDMA *dma, int is_write)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1146,6 +1150,24 @@ static int ahci_dma_prepare_buf(IDEDMA *dma, int 
is_write)
 return s->io_buffer_size != 0;
 }
 
+/**
+ * Destroys the scatter-gather list,
+ * and updates the command header with a bytes-read value.
+ * called explicitly via ahci_dma_rw_buf (ATAPI DMA),
+ * and ahci_start_transfer (PIO R/W),
+ * and called via callback from ide_dma_cb for DMA R/W paths.
+ */
+static void ahci_commit_buf(IDEDMA *dma, uint32_t tx_bytes)
+{
+AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
+IDEState *s = &ad->port.ifs[0];
+
+tx_bytes += le32_to_cpu(ad->cur_cmd->status);
+ad->cur_cmd->status = cpu_to_le32(tx_bytes);
+
+qemu_sglist_destroy(&s->sg);
+}
+
 static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1163,11 +1185,9 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
 dma_buf_write(p, l, &s->sg);
 }
 
-/* free sglist that was created in ahci_populate_sglist() */
-qemu_sglist_destroy(&s->sg);
+/* free sglist, update byte count */
+ahci_commit_buf(dma, l);
 
-/* update number of transferred bytes */
-ad->cur_cmd->status = cpu_to_le32(le32_to_cpu(ad->cur_cmd->status) + l);
 s->io_buffer_index += l;
 s->io_buffer_offset += l;
 
@@ -1210,6 +1230,7 @@ static const IDEDMAOps ahci_dma_ops = {
 .start_dma = ahci_start_dma,
 .start_transfer = ahci_start_transfer,
 .prepare_buf = ahci_dma_prepare_buf,
+.commit_buf = ahci_commit_buf,
 .rw_buf = ahci_dma_rw_buf,
 .set_unit = ahci_dma_set_unit,
 .cmd_done = ahci_cmd_done,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 190700a..f15b174 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -631,8 +631,11 @@ void ide_sector_read(IDEState *s)
   ide_sector_read_cb, s);
 }
 
-static void dma_buf_commit(IDEState *s)
+static void dma_buf_commit(IDEState *s, uint32_t tx_bytes)
 {
+if (s->bus->dma->ops->commit_buf) {
+s->bus->dma->ops->commit_buf(s->bus->dma, tx_bytes);
+}
 qemu_sglist_destroy(&s->sg);
 }
 
@@ -647,6 +650,7 @@ void ide_set_inactive(IDEState *s, bool more)
 
 void ide_dma_error(IDEState *s)
 {
+dma_buf_commit(s, 0);
 ide_abort_command(s);
 ide_set_inactive(s, false);
 ide_set_irq(s->bus);
@@ -662,7 +666,6 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 s->bus->error_st

[Qemu-devel] [PATCH 3/6] ide: repair PIO transfers for cases where nsector > 1

[John Snow]

Currently, for emulated PIO transfers through the AHCI device,
any attempt made to request more than a single sector's worth
of data will result in the same sector being transferred over
and over.

For example, if we request 8 sectors via PIO READ SECTORS, the
AHCI device will give us the same sector eight times.

This patch adds offset tracking into the PIO pathways so that
we can fulfill these requests appropriately.

Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 2 +-
 hw/ide/core.c | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 6966a94..db1d226 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1091,7 +1091,7 @@ static void ahci_start_transfer(IDEDMA *dma)
 goto out;
 }
 
-if (!ahci_populate_sglist(ad, &s->sg, 0)) {
+if (!ahci_populate_sglist(ad, &s->sg, s->io_buffer_offset)) {
 has_sglist = 1;
 }
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index f15b174..7c1929e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -589,6 +589,7 @@ static void ide_sector_read_cb(void *opaque, int ret)
 
 ide_set_sector(s, ide_get_sector(s) + n);
 s->nsector -= n;
+s->io_buffer_offset += 512 * n;
 }
 
 void ide_sector_read(IDEState *s)
@@ -829,6 +830,8 @@ static void ide_sector_write_cb(void *opaque, int ret)
 n = s->req_nb_sectors;
 }
 s->nsector -= n;
+s->io_buffer_offset += 512 * n;
+
 if (s->nsector == 0) {
 /* no more sectors to write */
 ide_transfer_stop(s);
@@ -1230,6 +1233,7 @@ static bool cmd_read_pio(IDEState *s, uint8_t cmd)
 
 ide_cmd_lba48_transform(s, lba48);
 s->req_nb_sectors = 1;
+s->io_buffer_offset = 0;
 ide_sector_read(s);
 
 return false;
@@ -1247,6 +1251,7 @@ static bool cmd_write_pio(IDEState *s, uint8_t cmd)
 ide_cmd_lba48_transform(s, lba48);
 
 s->req_nb_sectors = 1;
+s->io_buffer_offset = 0;
 s->status = SEEK_STAT | READY_STAT;
 ide_transfer_start(s, s->io_buffer, 512, ide_sector_write);
 
-- 
1.9.3

[Qemu-devel] [PATCH 0/6] AHCI Device Fixes

[John Snow]

Based off of feedback from the RFC of the same name,
this series batches together a group of fixes that
improve the AHCI device to fix a number of bugs.

A number of fixes included in the RFC that provide more
radical changes are omitted for now in favor of a smaller,
more easily reviewable set for QEMU 2.2.

In summary:

Patch #1 and #6 correct the format of FIS packet responses
that are available to the guest operating system upon interrupt.

Patch #2 corrects an oversight where we do not inform the
guest how many bytes we've transferred. This is relied upon
for non-NCQ modes and in some early bootup and shutdown code.

Patch #5 corrects cases with malformed scatter-gather lists that
may cause leaks, or cause QEMU to hang in an allocation loop.

Patch #4 attempts to continue minimizing the divergence of the
multiple pathways through the AHCI device by re-using existing
callbacks.

Taken together, these patches should allow non-ncq operation
for Windows hosts, as well as enable hibernation for Windows 7.

Hibernation for Windows 8 and AHCI remains non-functional.

John Snow (6):
  ahci: Correct PIO/D2H FIS responses
  ahci: Update byte count after DMA completion
  ide: repair PIO transfers for cases where nsector > 1
  ahci: unify sglist preparation
  ide: Correct handling of malformed/short PRDTs
  ahci: Fix SDB FIS Construction

 dma-helpers.c |   3 ++
 hw/ide/ahci.c | 113 --
 hw/ide/ahci.h |   8 
 hw/ide/core.c |  17 ++--
 hw/ide/internal.h |   2 +
 hw/ide/pci.c  |   5 ++-
 6 files changed, 97 insertions(+), 51 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver

[minyard]

From: Corey Minyard 

This keeps them from having to be passed around and makes them
available for later functions, like printing and reconnecting.

Signed-off-by: Corey Minyard 
Reviewed-by: Paolo Bonzini 
---
 qemu-char.c | 65 -
 1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 7928a4b..cd98911 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -28,6 +28,9 @@
 #include "sysemu/char.h"
 #include "hw/usb.h"
 #include "qmp-commands.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi-visit.h"
 
 #include 
 #include 
@@ -87,6 +90,34 @@
 #define CHR_MAX_FILENAME_SIZE 256
 
 /***/
+/* Socket address helpers */
+static void qapi_copy_SocketAddress(SocketAddress **p_dest,
+   SocketAddress *src)
+{
+QmpOutputVisitor *qov;
+QmpInputVisitor *qiv;
+Visitor *ov, *iv;
+QObject *obj;
+
+*p_dest = NULL;
+
+qov = qmp_output_visitor_new();
+ov = qmp_output_get_visitor(qov);
+visit_type_SocketAddress(ov, &src, NULL, &error_abort);
+obj = qmp_output_get_qobject(qov);
+qmp_output_visitor_cleanup(qov);
+if (!obj) {
+return;
+}
+
+qiv = qmp_input_visitor_new(obj);
+iv = qmp_input_get_visitor(qiv);
+visit_type_SocketAddress(iv, p_dest, NULL, &error_abort);
+qmp_input_visitor_cleanup(qiv);
+qobject_decref(obj);
+}
+
+/***/
 /* character device */
 
 static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
@@ -2412,6 +2443,10 @@ typedef struct {
 int read_msgfds_num;
 int *write_msgfds;
 int write_msgfds_num;
+
+SocketAddress *addr;
+bool is_listen;
+bool is_telnet;
 } TCPCharDriver;
 
 static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void 
*opaque);
@@ -2861,6 +2896,8 @@ static void tcp_chr_close(CharDriverState *chr)
 {
 TCPCharDriver *s = chr->opaque;
 int i;
+
+qapi_free_SocketAddress(s->addr);
 if (s->fd >= 0) {
 remove_fd_in_watch(chr);
 if (s->chan) {
@@ -2892,7 +2929,6 @@ static void tcp_chr_close(CharDriverState *chr)
 }
 
 static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
-  bool is_listen, bool is_telnet,
   Error **errp)
 {
 TCPCharDriver *s = chr->opaque;
@@ -2913,7 +2949,7 @@ static bool 
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
 case AF_UNIX:
 snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
  ((struct sockaddr_un *)(&ss))->sun_path,
- is_listen ? ",server" : "");
+ s->is_listen ? ",server" : "");
 break;
 #endif
 case AF_INET6:
@@ -2924,13 +2960,13 @@ static bool 
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
 getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
 serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
 snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
- is_telnet ? "telnet" : "tcp",
+ s->is_telnet ? "telnet" : "tcp",
  left, host, right, serv,
- is_listen ? ",server" : "");
+ s->is_listen ? ",server" : "");
 break;
 }
 
-if (is_listen) {
+if (s->is_listen) {
 s->listen_fd = fd;
 s->listen_chan = io_channel_from_socket(s->listen_fd);
 s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN,
@@ -2946,23 +2982,21 @@ static bool 
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
 return chr;
 }
 
-static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress *addr,
-bool is_listen, bool is_telnet,
-Error **errp)
+static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
 {
+TCPCharDriver *s = chr->opaque;
 int fd;
 
-if (is_listen) {
-fd = socket_listen(addr, errp);
+if (s->is_listen) {
+fd = socket_listen(s->addr, errp);
 } else  {
-fd = socket_connect(addr, errp, NULL, NULL);
+fd = socket_connect(s->addr, errp, NULL, NULL);
 }
 if (fd < 0) {
 return false;
 }
 
-return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
- errp);
+return qemu_chr_finish_socket_connection(chr, fd, errp);
 }
 
 /*/
@@ -3967,7 +4001,10 @@ static CharDriverState 
*qmp_chardev_open_socket(ChardevSocket *sock,
 s->fd = -1;
 s->listen_fd = -1;
 s->is_unix = addr->kind == SOCKET_ADDRESS_KIND_UNIX;
+s->is_listen = is_listen;
+s->is_telnet = is_te

[Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[minyard]

From: Corey Minyard 

Adds a "reconnect" option to socket backends that gives a reconnect
timeout.  This only applies to client sockets.  If the other end
of a socket closes the connection, qemu will attempt to reconnect
after the given number of seconds.

Signed-off-by: Corey Minyard 
---
 qapi-schema.json | 15 ++
 qemu-char.c  | 88 
 qemu-options.hx  | 20 -
 3 files changed, 105 insertions(+), 18 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 4bfaf20..148097b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2651,14 +2651,19 @@
 # @nodelay: #optional set TCP_NODELAY socket option (default: false)
 # @telnet: #optional enable telnet protocol on server
 #  sockets (default: false)
+# @reconnect: #optional For a client socket, if a socket is disconnected,
+#  then attempt a reconnect after the given number of seconds.
+#  Setting this to zero disables this function. (default: 0)
+#  (Since: 2.2)
 #
 # Since: 1.4
 ##
-{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
- '*server'  : 'bool',
- '*wait': 'bool',
- '*nodelay' : 'bool',
- '*telnet'  : 'bool' } }
+{ 'type': 'ChardevSocket', 'data': { 'addr'   : 'SocketAddress',
+ '*server': 'bool',
+ '*wait'  : 'bool',
+ '*nodelay'   : 'bool',
+ '*telnet': 'bool',
+ '*reconnect' : 'int' } }
 
 ##
 # @ChardevUdp:
diff --git a/qemu-char.c b/qemu-char.c
index b118e88..f33173c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2501,8 +2501,20 @@ typedef struct {
 SocketAddress *addr;
 bool is_listen;
 bool is_telnet;
+
+guint reconnect_timer;
+int64_t reconnect_time;
 } TCPCharDriver;
 
+static gboolean socket_reconnect_timeout(gpointer opaque);
+
+static void qemu_chr_socket_restart_timer(CharDriverState *chr)
+{
+TCPCharDriver *s = chr->opaque;
+s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time,
+   socket_reconnect_timeout, chr);
+}
+
 static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void 
*opaque);
 
 #ifndef _WIN32
@@ -2784,6 +2796,9 @@ static void tcp_chr_disconnect(CharDriverState *chr)
 SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
  "disconnected:", s->addr, s->is_listen, s->is_telnet);
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+if (s->reconnect_time) {
+qemu_chr_socket_restart_timer(chr);
+}
 }
 
 static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
@@ -2964,6 +2979,10 @@ static void tcp_chr_close(CharDriverState *chr)
 TCPCharDriver *s = chr->opaque;
 int i;
 
+if (s->reconnect_timer) {
+g_source_remove(s->reconnect_timer);
+s->reconnect_timer = 0;
+}
 qapi_free_SocketAddress(s->addr);
 if (s->fd >= 0) {
 remove_fd_in_watch(chr);
@@ -3013,7 +3032,28 @@ static bool 
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
 tcp_chr_connect(chr);
 }
 
-return chr;
+return true;
+}
+
+static void qemu_chr_socket_connected(int fd, void *opaque)
+{
+CharDriverState *chr = opaque;
+TCPCharDriver *s = chr->opaque;
+Error *err = NULL;
+
+if (fd >= 0) {
+if (qemu_chr_finish_socket_connection(chr, fd, &err)) {
+return;
+}
+if (err) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
+}
+closesocket(fd);
+}
+
+s->connected = 0;
+qemu_chr_socket_restart_timer(chr);
 }
 
 static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
@@ -3023,7 +3063,10 @@ static bool qemu_chr_open_socket_fd(CharDriverState 
*chr, Error **errp)
 
 if (s->is_listen) {
 fd = socket_listen(s->addr, errp);
-} else  {
+} else if (s->reconnect_time) {
+fd = socket_connect(s->addr, errp, qemu_chr_socket_connected, chr);
+return fd >= 0;
+} else {
 fd = socket_connect(s->addr, errp, NULL, NULL);
 }
 if (fd < 0) {
@@ -3450,6 +3493,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, 
ChardevBackend *backend,
 bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
 bool is_telnet  = qemu_opt_get_bool(opts, "telnet", false);
 bool do_nodelay = !qemu_opt_get_bool(opts, "delay", true);
+int64_t reconnect   = qemu_opt_get_number(opts, "reconnect", 0);
 const char *path = qemu_opt_get(opts, "path");
 const char *host = qemu_opt_get(opts, "host");
 const char *port = qemu_opt_get(opts, "port");
@@ -3476,6 +3520,8 @@ stati

[Qemu-devel] [PATCH v4 0/6] Add reconnect capability to sockets

[minyard]

This fixes some tab damage from the previous set.  That's it.

-corey

[Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect

[minyard]

From: Corey Minyard 

Move all socket configuration to qmp_chardev_open_socket().
qemu_chr_open_socket_fd() just opens the socket.  This is getting ready
for the reconnect code, which will call open_sock_fd() on a reconnect
attempt.

Signed-off-by: Corey Minyard 
Reviewed-by: Paolo Bonzini 
---
 qemu-char.c | 118 ++--
 1 file changed, 68 insertions(+), 50 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index f9d2a02..7928a4b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2891,13 +2891,11 @@ static void tcp_chr_close(CharDriverState *chr)
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
-static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
-bool is_listen, bool is_telnet,
-bool is_waitconnect,
-Error **errp)
+static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
+  bool is_listen, bool is_telnet,
+  Error **errp)
 {
-CharDriverState *chr = NULL;
-TCPCharDriver *s = NULL;
+TCPCharDriver *s = chr->opaque;
 char host[NI_MAXHOST], serv[NI_MAXSERV];
 const char *left = "", *right = "";
 struct sockaddr_storage ss;
@@ -2905,26 +2903,14 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 
 memset(&ss, 0, ss_len);
 if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
+closesocket(fd);
 error_setg_errno(errp, errno, "getsockname");
-return NULL;
+return false;
 }
 
-chr = qemu_chr_alloc();
-s = g_malloc0(sizeof(TCPCharDriver));
-
-s->connected = 0;
-s->fd = -1;
-s->listen_fd = -1;
-s->read_msgfds = 0;
-s->read_msgfds_num = 0;
-s->write_msgfds = 0;
-s->write_msgfds_num = 0;
-
-chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
 switch (ss.ss_family) {
 #ifndef _WIN32
 case AF_UNIX:
-s->is_unix = 1;
 snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
  ((struct sockaddr_un *)(&ss))->sun_path,
  is_listen ? ",server" : "");
@@ -2935,7 +2921,6 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 right = "]";
 /* fall through */
 case AF_INET:
-s->do_nodelay = do_nodelay;
 getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
 serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
 snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
@@ -2945,25 +2930,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 break;
 }
 
-chr->opaque = s;
-chr->chr_write = tcp_chr_write;
-chr->chr_sync_read = tcp_chr_sync_read;
-chr->chr_close = tcp_chr_close;
-chr->get_msgfds = tcp_get_msgfds;
-chr->set_msgfds = tcp_set_msgfds;
-chr->chr_add_client = tcp_chr_add_client;
-chr->chr_add_watch = tcp_chr_add_watch;
-chr->chr_update_read_handler = tcp_chr_update_read_handler;
-/* be isn't opened until we get a connection */
-chr->explicit_be_open = true;
-
 if (is_listen) {
 s->listen_fd = fd;
 s->listen_chan = io_channel_from_socket(s->listen_fd);
-s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, 
tcp_chr_accept, chr);
-if (is_telnet) {
-s->do_telnetopt = 1;
-}
+s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN,
+   tcp_chr_accept, chr);
 } else {
 s->connected = 1;
 s->fd = fd;
@@ -2972,15 +2943,28 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 tcp_chr_connect(chr);
 }
 
-if (is_listen && is_waitconnect) {
-fprintf(stderr, "QEMU waiting for connection on: %s\n",
-chr->filename);
-tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
-qemu_set_nonblock(s->listen_fd);
-}
 return chr;
 }
 
+static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress *addr,
+bool is_listen, bool is_telnet,
+Error **errp)
+{
+int fd;
+
+if (is_listen) {
+fd = socket_listen(addr, errp);
+} else  {
+fd = socket_connect(addr, errp, NULL, NULL);
+}
+if (fd < 0) {
+return false;
+}
+
+return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
+ errp);
+}
+
 /*/
 /* Ring buffer chardev */
 
@@ -3969,23 +3953,57 @@ static CharDriverState 
*qmp_chardev_open_parallel(ChardevHostdev *parallel,
 static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,

[Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket

[minyard]

From: Corey Minyard 

It seems that it might be a good idea to know what is at the remote
end of a socket for tracking down issues.  So add that to the
socket filename.

Signed-off-by: Corey Minyard 
Reviewed-by: Paolo Bonzini 
---
 qemu-char.c | 27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index f33173c..96dddec 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -142,9 +142,11 @@ static int SocketAddress_to_str(char *dest, int max_len,
 
 static int sockaddr_to_str(char *dest, int max_len,
struct sockaddr_storage *ss, socklen_t ss_len,
+   struct sockaddr_storage *ps, socklen_t ps_len,
bool is_listen, bool is_telnet)
 {
-char host[NI_MAXHOST], serv[NI_MAXSERV];
+char shost[NI_MAXHOST], sserv[NI_MAXSERV];
+char phost[NI_MAXHOST], pserv[NI_MAXSERV];
 const char *left = "", *right = "";
 
 switch (ss->ss_family) {
@@ -159,12 +161,15 @@ static int sockaddr_to_str(char *dest, int max_len,
 right = "]";
 /* fall through */
 case AF_INET:
-getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
-serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
-return snprintf(dest, max_len, "%s:%s%s%s:%s%s",
+getnameinfo((struct sockaddr *) ss, ss_len, shost, sizeof(shost),
+sserv, sizeof(sserv), NI_NUMERICHOST | NI_NUMERICSERV);
+getnameinfo((struct sockaddr *) ps, ps_len, phost, sizeof(phost),
+pserv, sizeof(pserv), NI_NUMERICHOST | NI_NUMERICSERV);
+return snprintf(dest, max_len, "%s:%s%s%s:%s%s <-> %s%s%s:%s",
 is_telnet ? "telnet" : "tcp",
-left, host, right, serv,
-is_listen ? ",server" : "");
+left, shost, right, sserv,
+is_listen ? ",server" : "",
+left, phost, right, pserv);
 
 default:
 return snprintf(dest, max_len, "unknown");
@@ -2869,15 +2874,19 @@ static void tcp_chr_connect(void *opaque)
 {
 CharDriverState *chr = opaque;
 TCPCharDriver *s = chr->opaque;
-struct sockaddr_storage ss;
-socklen_t ss_len = sizeof(ss);
+struct sockaddr_storage ss, ps;
+socklen_t ss_len = sizeof(ss), ps_len = sizeof(ps);
 
 memset(&ss, 0, ss_len);
 if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) {
 snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
  "Error in getsockname: %s\n", strerror(errno));
+} else if (getpeername(s->fd, (struct sockaddr *) &ps, &ps_len) != 0) {
+snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
+ "Error in getpeername: %s\n", strerror(errno));
 } else {
-sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len,
+sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
+&ss, ss_len, &ps, ps_len,
 s->is_listen, s->is_telnet);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected

[minyard]

From: Corey Minyard 

This way we can tell if the socket is connected or not.  It also splits
the string conversions out into separate functions to make this more
convenient.

Signed-off-by: Corey Minyard 
Reviewed-by: Paolo Bonzini 
---
 qemu-char.c | 102 
 1 file changed, 69 insertions(+), 33 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index cd98911..b118e88 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -117,6 +117,60 @@ static void qapi_copy_SocketAddress(SocketAddress **p_dest,
 qobject_decref(obj);
 }
 
+static int SocketAddress_to_str(char *dest, int max_len,
+const char *prefix, SocketAddress *addr,
+bool is_listen, bool is_telnet)
+{
+switch (addr->kind) {
+case SOCKET_ADDRESS_KIND_INET:
+return snprintf(dest, max_len, "%s%s:%s:%s%s", prefix,
+is_telnet ? "telnet" : "tcp", addr->inet->host,
+addr->inet->port, is_listen ? ",server" : "");
+break;
+case SOCKET_ADDRESS_KIND_UNIX:
+return snprintf(dest, max_len, "%sunix:%s%s", prefix,
+addr->q_unix->path, is_listen ? ",server" : "");
+break;
+case SOCKET_ADDRESS_KIND_FD:
+return snprintf(dest, max_len, "%sfd:%s%s", prefix, addr->fd->str,
+is_listen ? ",server" : "");
+break;
+default:
+abort();
+}
+}
+
+static int sockaddr_to_str(char *dest, int max_len,
+   struct sockaddr_storage *ss, socklen_t ss_len,
+   bool is_listen, bool is_telnet)
+{
+char host[NI_MAXHOST], serv[NI_MAXSERV];
+const char *left = "", *right = "";
+
+switch (ss->ss_family) {
+#ifndef _WIN32
+case AF_UNIX:
+return snprintf(dest, max_len, "unix:%s%s",
+((struct sockaddr_un *)(ss))->sun_path,
+is_listen ? ",server" : "");
+#endif
+case AF_INET6:
+left  = "[";
+right = "]";
+/* fall through */
+case AF_INET:
+getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
+serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
+return snprintf(dest, max_len, "%s:%s%s%s:%s%s",
+is_telnet ? "telnet" : "tcp",
+left, host, right, serv,
+is_listen ? ",server" : "");
+
+default:
+return snprintf(dest, max_len, "unknown");
+}
+}
+
 /***/
 /* character device */
 
@@ -2727,6 +2781,8 @@ static void tcp_chr_disconnect(CharDriverState *chr)
 s->chan = NULL;
 closesocket(s->fd);
 s->fd = -1;
+SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
+ "disconnected:", s->addr, s->is_listen, s->is_telnet);
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
@@ -2798,6 +2854,17 @@ static void tcp_chr_connect(void *opaque)
 {
 CharDriverState *chr = opaque;
 TCPCharDriver *s = chr->opaque;
+struct sockaddr_storage ss;
+socklen_t ss_len = sizeof(ss);
+
+memset(&ss, 0, ss_len);
+if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) {
+snprintf(chr->filename, CHR_MAX_FILENAME_SIZE,
+ "Error in getsockname: %s\n", strerror(errno));
+} else {
+sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len,
+s->is_listen, s->is_telnet);
+}
 
 s->connected = 1;
 if (s->chan) {
@@ -2932,39 +2999,6 @@ static bool 
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
   Error **errp)
 {
 TCPCharDriver *s = chr->opaque;
-char host[NI_MAXHOST], serv[NI_MAXSERV];
-const char *left = "", *right = "";
-struct sockaddr_storage ss;
-socklen_t ss_len = sizeof(ss);
-
-memset(&ss, 0, ss_len);
-if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
-closesocket(fd);
-error_setg_errno(errp, errno, "getsockname");
-return false;
-}
-
-switch (ss.ss_family) {
-#ifndef _WIN32
-case AF_UNIX:
-snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
- ((struct sockaddr_un *)(&ss))->sun_path,
- s->is_listen ? ",server" : "");
-break;
-#endif
-case AF_INET6:
-left  = "[";
-right = "]";
-/* fall through */
-case AF_INET:
-getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
-serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
-snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
- s->is_telnet ? "telnet" : "tcp",
- left, host, right, serv,
- s->is_listen ? ",server" : "");
-break;
-}
 
 if (s->is_listen) {
 s->lis

[Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define

[minyard]

From: Corey Minyard 

Signed-off-by: Corey Minyard 
Reviewed-by: Paolo Bonzini 
---
 qemu-char.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8623c70..f9d2a02 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -84,6 +84,7 @@
 
 #define READ_BUF_LEN 4096
 #define READ_RETRIES 10
+#define CHR_MAX_FILENAME_SIZE 256
 
 /***/
 /* character device */
@@ -989,7 +990,8 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int 
fd_out)
 static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
 {
 int fd_in, fd_out;
-char filename_in[256], filename_out[256];
+char filename_in[CHR_MAX_FILENAME_SIZE];
+char filename_out[CHR_MAX_FILENAME_SIZE];
 const char *filename = opts->device;
 
 if (filename == NULL) {
@@ -997,8 +999,8 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev 
*opts)
 return NULL;
 }
 
-snprintf(filename_in, 256, "%s.in", filename);
-snprintf(filename_out, 256, "%s.out", filename);
+snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
+snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename);
 TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
 TFR(fd_out = qemu_open(filename_out, O_RDWR | O_BINARY));
 if (fd_in < 0 || fd_out < 0) {
@@ -1976,7 +1978,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const 
char *filename)
 OVERLAPPED ov;
 int ret;
 DWORD size;
-char openname[256];
+char openname[CHR_MAX_FILENAME_SIZE];
 
 s->fpipe = TRUE;
 
@@ -2918,12 +2920,12 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 s->write_msgfds = 0;
 s->write_msgfds_num = 0;
 
-chr->filename = g_malloc(256);
+chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE);
 switch (ss.ss_family) {
 #ifndef _WIN32
 case AF_UNIX:
 s->is_unix = 1;
-snprintf(chr->filename, 256, "unix:%s%s",
+snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s",
  ((struct sockaddr_un *)(&ss))->sun_path,
  is_listen ? ",server" : "");
 break;
@@ -2936,7 +2938,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 s->do_nodelay = do_nodelay;
 getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
 serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
-snprintf(chr->filename, 256, "%s:%s%s%s:%s%s",
+snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s",
  is_telnet ? "telnet" : "tcp",
  left, host, right, serv,
  is_listen ? ",server" : "");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[Corey Minyard]

On 10/01/2014 02:10 PM, Eric Blake wrote:
> On 09/25/2014 02:07 PM, miny...@acm.org wrote:
>> From: Corey Minyard 
>>
>> Adds a "reconnect" option to socket backends that gives a reconnect
>> timeout.  This only applies to client sockets.  If the other end
>> of a socket closes the connection, qemu will attempt to reconnect
>> after the given number of seconds.
>>
>> Signed-off-by: Corey Minyard 
>> ---
>>  qapi-schema.json | 15 ++
>>  qemu-char.c  | 88 
>> 
>>  qemu-options.hx  | 20 -
>>  3 files changed, 105 insertions(+), 18 deletions(-)
>>
>>  
>> +static gboolean socket_reconnect_timeout(gpointer opaque);
>> +
> Do you really need a forward declaration of this static function, or can
> you just stick the body of the function here?

It's a circular reference.  The timeout calls the function to restart
the timer, which
then references the timeout.

>
>> @@ -2964,6 +2979,10 @@ static void tcp_chr_close(CharDriverState *chr)
>>  TCPCharDriver *s = chr->opaque;
>>  int i;
>>  
>> +if (s->reconnect_timer) {
>> +g_source_remove(s->reconnect_timer);
>> +s->reconnect_timer = 0;
> TAB damage.

Dang, I thought I had fixed all those.  I'll do another round of patches
with
that and the unnecessary () removed.

-corey

Re: [Qemu-devel] Request for help

[Martin Townsend]

Hi Christopher,

Thanks for the reply, I will take a look at the Versatile Express code.

- Martin.


On 01/10/14 17:41, Christopher Covington wrote:

Hi Martin,

On 10/01/2014 09:50 AM, Martin Townsend wrote:

Hi,

I'm looking into creating a virtualised test bed for an 802.15.4 network.
Currently I have QEMU running and emulating our HW which bridges the
Ethernet from the guest to the host. What I would like to do is something
similar with the 802.15.4 network interface. The host doesn't have a
physical 802.15.4 interface but if I could get the data out of the guest
and into QEMU I could communicate with a central network simulator to deal
with the packets.

My first thought was to write a virtual device for QEMU that would
interface with our 802.15.4 driver in the guest but then I realise that the
real device hangs of an AMBA bus

What exactly is your objection to this? Aren't there plenty of "AMBA bus"
peripherals that you can reference attached to the Versatile Express for 
example?



so my next idea was to write a PCI driver for 802.15.4 on the guest making
it possible to write a PCI virtual QEMU device.

I just wanted to confirm that this is possible and whether there is a
better solution?

Christopher

[Qemu-devel] [PATCH 3/4] qemu-file: Move unix and socket implementations to qemu-file-unix.c

[Eduardo Habkost]

Separate the QEMUFile interface from the implementation, to reduce
dependencies from code using QEMUFile.

All the code that is being moved to the new file is exactly the same
code that was on savevm.c (moved by commit
093c455a8c6d8f715eabd8c8d346f08f17d686ec), so I am using the copyright
and license header from savevm.c for the new file.

Signed-off-by: Eduardo Habkost 
---
 Makefile.objs|   2 +-
 qemu-file-unix.c | 223 +++
 qemu-file.c  | 195 
 tests/Makefile   |   2 +-
 4 files changed, 225 insertions(+), 197 deletions(-)
 create mode 100644 qemu-file-unix.c

diff --git a/Makefile.objs b/Makefile.objs
index 97db978..b89134b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -50,7 +50,7 @@ common-obj-$(CONFIG_LINUX) += fsdev/
 
 common-obj-y += migration.o migration-tcp.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o
+common-obj-y += qemu-file.o qemu-file-unix.o
 common-obj-$(CONFIG_RDMA) += migration-rdma.o
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += block-migration.o
diff --git a/qemu-file-unix.c b/qemu-file-unix.c
new file mode 100644
index 000..9682396
--- /dev/null
+++ b/qemu-file-unix.c
@@ -0,0 +1,223 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-common.h"
+#include "qemu/iov.h"
+#include "qemu/sockets.h"
+#include "block/coroutine.h"
+#include "migration/qemu-file.h"
+
+typedef struct QEMUFileSocket {
+int fd;
+QEMUFile *file;
+} QEMUFileSocket;
+
+static ssize_t socket_writev_buffer(void *opaque, struct iovec *iov, int 
iovcnt,
+int64_t pos)
+{
+QEMUFileSocket *s = opaque;
+ssize_t len;
+ssize_t size = iov_size(iov, iovcnt);
+
+len = iov_send(s->fd, iov, iovcnt, 0, size);
+if (len < size) {
+len = -socket_error();
+}
+return len;
+}
+
+static int socket_get_fd(void *opaque)
+{
+QEMUFileSocket *s = opaque;
+
+return s->fd;
+}
+
+static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+QEMUFileSocket *s = opaque;
+ssize_t len;
+
+for (;;) {
+len = qemu_recv(s->fd, buf, size, 0);
+if (len != -1) {
+break;
+}
+if (socket_error() == EAGAIN) {
+yield_until_fd_readable(s->fd);
+} else if (socket_error() != EINTR) {
+break;
+}
+}
+
+if (len == -1) {
+len = -socket_error();
+}
+return len;
+}
+
+static int socket_close(void *opaque)
+{
+QEMUFileSocket *s = opaque;
+closesocket(s->fd);
+g_free(s);
+return 0;
+}
+
+static ssize_t unix_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
+  int64_t pos)
+{
+QEMUFileSocket *s = opaque;
+ssize_t len, offset;
+ssize_t size = iov_size(iov, iovcnt);
+ssize_t total = 0;
+
+assert(iovcnt > 0);
+offset = 0;
+while (size > 0) {
+/* Find the next start position; skip all full-sized vector elements  
*/
+while (offset >= iov[0].iov_len) {
+offset -= iov[0].iov_len;
+iov++, iovcnt--;
+}
+
+/* skip `offset' bytes from the (now) first element, undo it on exit */
+assert(iovcnt > 0);
+iov[0].iov_base += offset;
+iov[0].iov_len -= offset;
+
+do {
+len = writev(s->fd, iov, iovcnt);
+} while (len == -1 && errno == EINTR);
+if (len == -1) {
+return -errno;
+}
+
+/* Undo the changes above */
+iov[0].iov_base -= offset;
+iov[0].iov_len += offset;
+
+/* Prepare for the next iteration */
+offset += len;
+total += len;
+size -= len;
+}
+
+return total;
+}
+
+static int unix_get_buffer(void *opaque, uint8_t *buf, int64_t

[Qemu-devel] [PATCH 1/4] qemu-file: Make qemu_file_is_writable() non-static

[Eduardo Habkost]

The QEMUFileStdio code will use qemu_file_is_writable() and will be
moved to a separate file.

Signed-off-by: Eduardo Habkost 
---
 include/migration/qemu-file.h | 1 +
 qemu-file.c   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index c90f529..1885b41 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -110,6 +110,7 @@ void qemu_put_byte(QEMUFile *f, int v);
  */
 void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size);
 bool qemu_file_mode_is_not_valid(const char *mode);
+bool qemu_file_is_writable(QEMUFile *f);
 
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
diff --git a/qemu-file.c b/qemu-file.c
index a8e3912..6c8a6c9 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -433,7 +433,7 @@ void qemu_file_set_error(QEMUFile *f, int ret)
 }
 }
 
-static inline bool qemu_file_is_writable(QEMUFile *f)
+bool qemu_file_is_writable(QEMUFile *f)
 {
 return f->ops->writev_buffer || f->ops->put_buffer;
 }
-- 
1.9.3

[Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files

[Eduardo Habkost]

With this, code that uses symbols from qemu-file.c don't need to bring extra
dependencies because of the actual QEMUFile operation implementations.

Eduardo Habkost (4):
  qemu-file: Make qemu_file_is_writable() non-static
  qemu-file: Use qemu_file_is_writable() on stdio_fclose()
  qemu-file: Move unix and socket implementations to qemu-file-unix.c
  qemu-file: Move stdio implementation to qemu-file-stdio.c

 Makefile.objs |   2 +-
 include/migration/qemu-file.h |   1 +
 qemu-file-stdio.c | 194 ++
 qemu-file-unix.c  | 223 ++
 qemu-file.c   | 365 +-
 tests/Makefile|   2 +-
 6 files changed, 421 insertions(+), 366 deletions(-)
 create mode 100644 qemu-file-stdio.c
 create mode 100644 qemu-file-unix.c

-- 
1.9.3

[Qemu-devel] [PATCH 4/4] qemu-file: Move stdio implementation to qemu-file-stdio.c

[Eduardo Habkost]

Separate the QEMUFile interface from the stdio-specific implementation,
to reduce dependencies from code using QEMUFile.

The code that is being moved is similar to the one that was on savevm.c before
it was moved in commit 093c455a8c6d8f715eabd8c8d346f08f17d686ec, except for
some changes done by Markus, Juan, and myself. So, I am using the copyright and
license header from savevm.c, but CCing Juan and Markus so they can review the
copyright/license header.

Cc: Markus Armbruster 
Cc: Juan Quintela 
Signed-off-by: Eduardo Habkost 
---
 Makefile.objs |   2 +-
 qemu-file-stdio.c | 194 ++
 qemu-file.c   | 168 --
 3 files changed, 195 insertions(+), 169 deletions(-)
 create mode 100644 qemu-file-stdio.c

diff --git a/Makefile.objs b/Makefile.objs
index b89134b..22239a3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -50,7 +50,7 @@ common-obj-$(CONFIG_LINUX) += fsdev/
 
 common-obj-y += migration.o migration-tcp.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-unix.o
+common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-$(CONFIG_RDMA) += migration-rdma.o
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += block-migration.o
diff --git a/qemu-file-stdio.c b/qemu-file-stdio.c
new file mode 100644
index 000..285068b
--- /dev/null
+++ b/qemu-file-stdio.c
@@ -0,0 +1,194 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-common.h"
+#include "block/coroutine.h"
+#include "migration/qemu-file.h"
+
+typedef struct QEMUFileStdio {
+FILE *stdio_file;
+QEMUFile *file;
+} QEMUFileStdio;
+
+static int stdio_get_fd(void *opaque)
+{
+QEMUFileStdio *s = opaque;
+
+return fileno(s->stdio_file);
+}
+
+static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
+int size)
+{
+QEMUFileStdio *s = opaque;
+int res;
+
+res = fwrite(buf, 1, size, s->stdio_file);
+
+if (res != size) {
+return -errno;
+}
+return res;
+}
+
+static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
+{
+QEMUFileStdio *s = opaque;
+FILE *fp = s->stdio_file;
+int bytes;
+
+for (;;) {
+clearerr(fp);
+bytes = fread(buf, 1, size, fp);
+if (bytes != 0 || !ferror(fp)) {
+break;
+}
+if (errno == EAGAIN) {
+yield_until_fd_readable(fileno(fp));
+} else if (errno != EINTR) {
+break;
+}
+}
+return bytes;
+}
+
+static int stdio_pclose(void *opaque)
+{
+QEMUFileStdio *s = opaque;
+int ret;
+ret = pclose(s->stdio_file);
+if (ret == -1) {
+ret = -errno;
+} else if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
+/* close succeeded, but non-zero exit code: */
+ret = -EIO; /* fake errno value */
+}
+g_free(s);
+return ret;
+}
+
+static int stdio_fclose(void *opaque)
+{
+QEMUFileStdio *s = opaque;
+int ret = 0;
+
+if (qemu_file_is_writable(s->file)) {
+int fd = fileno(s->stdio_file);
+struct stat st;
+
+ret = fstat(fd, &st);
+if (ret == 0 && S_ISREG(st.st_mode)) {
+/*
+ * If the file handle is a regular file make sure the
+ * data is flushed to disk before signaling success.
+ */
+ret = fsync(fd);
+if (ret != 0) {
+ret = -errno;
+return ret;
+}
+}
+}
+if (fclose(s->stdio_file) == EOF) {
+ret = -errno;
+}
+g_free(s);
+return ret;
+}
+
+static const QEMUFileOps stdio_pipe_read_ops = {
+.get_fd = stdio_get_fd,
+.get_buffer = stdio_get_buffer,
+.close =  stdio_pclose
+};
+
+static const QEMUFileOps stdio_pipe_write_ops = 

[Qemu-devel] [PATCH 2/4] qemu-file: Use qemu_file_is_writable() on stdio_fclose()

[Eduardo Habkost]

Use the existing function which checks if writev_buffer() or
put_buffer() are set, instead of duplicating it.

Signed-off-by: Eduardo Habkost 
---
 qemu-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-file.c b/qemu-file.c
index 6c8a6c9..482bda6 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -152,7 +152,7 @@ static int stdio_fclose(void *opaque)
 QEMUFileStdio *s = opaque;
 int ret = 0;
 
-if (s->file->ops->put_buffer || s->file->ops->writev_buffer) {
+if (qemu_file_is_writable(s->file)) {
 int fd = fileno(s->stdio_file);
 struct stat st;
 
-- 
1.9.3

Re: [Qemu-devel] NBD TLS support in QEMU

[Wouter Verhelst]

Hi,

On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote:
> Tunneling the entire protocol inside an SSL connection doesn't fix that;
> if an attacker is able to hijack your TCP connections and change flags,
> then this attacker is also able to hijack your TCP connection and
> redirect it to a decrypting/encrypting proxy.
> 
> I agree that preventing a possible SSL downgrade attack (and other forms
> of MITM) should be high on the priority list, but "tunnel the whole
> thing in SSL" doesn't do that.

So, having given this some thought, I wanted to come up with a spec just
so that we had something we could all agree on. As part of that, I had a
look at qemu-nbd, and noticed that it uses the "oldstyle" handshake
protocol (on port 10809 by default -- ew, please don't do that).

I had to change the protocol incompatibly a few years back, because the
oldstyle protocol is broken by design; in the oldstyle negotiation
protocol, the server dumps all information it has on the export to the
client, and then moves on to the data negotiation phase, without waiting
for any reply from the client. This means the oldstyle protocol can't be
used for any sort of negotiation[1].

As such, I strongly suggest that qemu-nbd move to the newstyle protocol.
This would also allow you to use named exports and various other things,
and would allow negotiation of TLS or SASL in a clean and proper way.

[1] That sole issue is the reason I broke backwards compatibility with
the newstyle handshake protocol, and is also why I reserved 10809,
the port assigned to nbd by IANA at my request, to be for newstyle
handshakes only.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[Eric Blake]

On 09/25/2014 02:07 PM, miny...@acm.org wrote:
> From: Corey Minyard 
> 
> Adds a "reconnect" option to socket backends that gives a reconnect
> timeout.  This only applies to client sockets.  If the other end
> of a socket closes the connection, qemu will attempt to reconnect
> after the given number of seconds.
> 
> Signed-off-by: Corey Minyard 
> ---
>  qapi-schema.json | 15 ++
>  qemu-char.c  | 88 
> 
>  qemu-options.hx  | 20 -
>  3 files changed, 105 insertions(+), 18 deletions(-)
> 

>  
> +static gboolean socket_reconnect_timeout(gpointer opaque);
> +

Do you really need a forward declaration of this static function, or can
you just stick the body of the function here?


> @@ -2964,6 +2979,10 @@ static void tcp_chr_close(CharDriverState *chr)
>  TCPCharDriver *s = chr->opaque;
>  int i;
>  
> +if (s->reconnect_timer) {
> +g_source_remove(s->reconnect_timer);
> + s->reconnect_timer = 0;

TAB damage.


> @@ -3023,7 +3063,10 @@ static bool qemu_chr_open_socket_fd(CharDriverState 
> *chr, Error **errp)
>  
>  if (s->is_listen) {
>  fd = socket_listen(s->addr, errp);
> -} else  {
> +} else if (s->reconnect_time) {
> + fd = socket_connect(s->addr, errp, qemu_chr_socket_connected, chr);
> + return (fd >= 0);

More TAB damage.  Also, the () in the return are not necessary.

-- 
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 07/11] block: let backup blockjob run in BDS AioContext

[Max Reitz]

On 01.10.2014 19:01, Stefan Hajnoczi wrote:

The backup block job must run in the BlockDriverState AioContext so that
it works with dataplane.

The basics of acquiring the AioContext are easy in blockdev.c.

The completion code in block/backup.c must call bdrv_unref() from the
main loop.  Use block_job_defer_to_main_loop() to achieve that.

Signed-off-by: Stefan Hajnoczi 
---
  block/backup.c | 21 +++--
  blockdev.c | 23 ---
  2 files changed, 35 insertions(+), 9 deletions(-)


Hm, so, if I run a backup blockjob from one thread (which is not the 
main loop and not the BDS's AIO context) and try to cancel it or set its 
speed from another thread (e.g. the main loop), won't the blockjob hold 
the BDS's AIO context lock as long as it runs and making it impossible 
to interfere?


Max

Re: [Qemu-devel] [PATCH 06/11] block: add bdrv_drain()

[Max Reitz]

On 01.10.2014 19:01, Stefan Hajnoczi wrote:

Now that op blockers are in use, we can ensure that no other sources are
generating I/O on a BlockDriverState.  Therefore it is possible to drain
requests for a single BDS.

Signed-off-by: Stefan Hajnoczi 
---
  block.c   | 36 +---
  include/block/block.h |  1 +
  2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index c5a251c..48305c4 100644
--- a/block.c
+++ b/block.c
@@ -1918,6 +1918,34 @@ static bool bdrv_requests_pending(BlockDriverState *bs)
  return false;
  }
  
+static bool bdrv_drain_one(BlockDriverState *bs)

+{
+bool bs_busy;
+
+bdrv_flush_io_queue(bs);
+bdrv_start_throttled_reqs(bs);
+bs_busy = bdrv_requests_pending(bs);
+bs_busy |= aio_poll(bdrv_get_aio_context(bs), bs_busy);
+return bs_busy;
+}
+
+/*
+ * Wait for pending requests to complete on a single BlockDriverState subtree
+ *
+ * See the warning in bdrv_drain_all().  This function can only be called if
+ * you are sure nothing can generate I/O because you have op blockers
+ * installed.


Although that warning now sounds too harsh: "it is not possible to have 
a function to drain a single device's I/O queue." Apparently, under 
certain circumstances, it now *is* possible. ;-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 05/11] blockjob: add block_job_defer_to_main_loop()

[Max Reitz]

On 01.10.2014 19:01, Stefan Hajnoczi wrote:

Block jobs will run in the BlockDriverState's AioContext, which may not
always be the QEMU main loop.

There are some block layer APIs that are either not thread-safe or risk
lock ordering problems.  This includes bdrv_unref(), bdrv_close(), and
anything that calls bdrv_drain_all().

The block_job_defer_to_main_loop() API allows a block job to schedule a
function to run in the main loop with the BlockDriverState AioContext
held.

This function will be used to perform cleanup and backing chain
manipulations in block jobs.

Signed-off-by: Stefan Hajnoczi 
---
  blockjob.c   | 35 +++
  include/block/blockjob.h | 19 +++
  2 files changed, 54 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 0689fdd..24a64d8 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -313,3 +313,38 @@ BlockErrorAction block_job_error_action(BlockJob *job, 
BlockDriverState *bs,
  }
  return action;
  }
+
+typedef struct {
+BlockJob *job;
+QEMUBH *bh;
+AioContext *aio_context;
+BlockJobDeferToMainLoopFn *fn;
+void *opaque;
+} BlockJobDeferToMainLoopData;
+
+static void block_job_defer_to_main_loop_bh(void *opaque)
+{
+BlockJobDeferToMainLoopData *data = opaque;
+
+qemu_bh_delete(data->bh);
+
+aio_context_acquire(data->aio_context);
+data->fn(data->job, data->opaque);
+aio_context_release(data->aio_context);
+
+g_free(data);
+}
+
+void block_job_defer_to_main_loop(BlockJob *job,
+  BlockJobDeferToMainLoopFn *fn,
+  void *opaque)
+{
+BlockJobDeferToMainLoopData *data = g_malloc(sizeof(*data));
+data->job = job;
+data->bh = qemu_bh_new(block_job_defer_to_main_loop_bh, data);
+data->aio_context = bdrv_get_aio_context(job->bs);
+data->fn = fn;
+data->opaque = opaque;
+
+qemu_bh_schedule(data->bh);
+}


I'm not so sure whether it'd be possible to change the BDS's AIO context 
(in another thread) after the call to bdrv_get_aio_context() in 
block_job_defer_to_main_loop() and before 
block_job_defer_to_main_loop_bh() is run. If so, this might create race 
conditions.


Other than that, this patch looks good.

Max

Re: [Qemu-devel] [PATCH 04/11] blockdev: add note that block_job_cb() must be thread-safe

[Max Reitz]

On 01.10.2014 19:01, Stefan Hajnoczi wrote:

This function is correct but we should document the constraint that
everything must be thread-safe.

Emitting QMP events and scheduling BHs are both thread-safe so nothing
needs to be done here.

Signed-off-by: Stefan Hajnoczi 
---
  blockdev.c | 5 +
  1 file changed, 5 insertions(+)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 03/11] blockdev: acquire AioContext in blockdev_mark_auto_del()

[Max Reitz]

On 01.10.2014 19:01, Stefan Hajnoczi wrote:

When an emulated storage controller is unrealized it will call
blockdev_mark_auto_del().  This will cancel any running block job (and
that eventually releases its reference to the BDS so it can be freed).

Since the block job may be executing in another AioContext we must
acquire/release to ensure thread safety.

Signed-off-by: Stefan Hajnoczi 
---
  blockdev.c | 7 +++
  1 file changed, 7 insertions(+)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 02/11] blockdev: acquire AioContext in do_qmp_query_block_jobs_one()

[Max Reitz]

On 01.10.2014 19:01, Stefan Hajnoczi wrote:

Make sure that query-block-jobs acquires the BlockDriverState
AioContext so that the blockjob isn't running in another thread while we
access its state.

Signed-off-by: Stefan Hajnoczi 
---
  blockdev.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)


This conflicts with patch 7 Markus's BlockBackend series; but in itself, 
this patch is fine.


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH 01/11] block: acquire AioContext in generic blockjob QMP commands

[Max Reitz]

On 01.10.2014 19:01, Stefan Hajnoczi wrote:

block-job-set-speed, block-job-cancel, block-job-pause,
block-job-resume, and block-job-complete must acquire the
BlockDriverState AioContext so that it is safe to access bs.

At the moment bs->job is always NULL when dataplane is active because op
blockers prevent blockjobs from starting.  Once the rest of the blockjob
API has been made aware of AioContext we can drop the op blocker.

Signed-off-by: Stefan Hajnoczi 
---
  blockdev.c | 52 +++-
  1 file changed, 39 insertions(+), 13 deletions(-)


Reviewed-by: Max Reitz 

[Qemu-devel] [PATCH v3 1/6] blockdev: Orphaned drive search

[John Snow]

When users use command line options like -hda, -cdrom,
or even -drive if=ide, it is up to the board initialization
routines to pick up these drives and create backing
devices for them.

Some boards, like Q35, have not been doing this.
However, there is no warning explaining why certain
drive specifications are just silently ignored,
so this function adds a check to print some warnings
to assist users in debugging these sorts of issues
in the future.

This patch will not warn about drives added with if_none,
for which it is not possible to tell in advance if
the omission of a backing device is an issue.

A warning in these cases is considered appropriate.

Signed-off-by: John Snow 
---
 blockdev.c| 21 +
 include/sysemu/blockdev.h |  2 ++
 vl.c  | 10 +-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index ad43648..d2ad065 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -166,6 +166,27 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int 
unit)
 return NULL;
 }
 
+bool drive_check_orphaned(void)
+{
+DriveInfo *dinfo;
+bool rs = false;
+
+QTAILQ_FOREACH(dinfo, &drives, next) {
+/* If dinfo->bdrv->dev is NULL, it has no device attached. */
+/* Unless this is a default drive, this may be an oversight. */
+if (!dinfo->bdrv->dev && !dinfo->is_default &&
+dinfo->type != IF_NONE) {
+fprintf(stderr, "Warning: Orphaned drive without device: "
+"id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
+dinfo->id, dinfo->bdrv->filename, if_name[dinfo->type],
+dinfo->bus, dinfo->unit);
+rs = true;
+}
+}
+
+return rs;
+}
+
 DriveInfo *drive_get_by_index(BlockInterfaceType type, int index)
 {
 return drive_get(type,
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index abec381..3040286 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -38,6 +38,7 @@ struct DriveInfo {
 int unit;
 int auto_del;   /* see blockdev_mark_auto_del() */
 bool enable_auto_del;   /* Only for legacy drive_new() */
+bool is_default;/* Added by default_drive() ?  */
 int media_cd;
 int cyls, heads, secs, trans;
 QemuOpts *opts;
@@ -46,6 +47,7 @@ struct DriveInfo {
 };
 
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
+bool drive_check_orphaned(void);
 DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
 int drive_get_max_bus(BlockInterfaceType type);
 DriveInfo *drive_get_next(BlockInterfaceType type);
diff --git a/vl.c b/vl.c
index dbdca59..6500472 100644
--- a/vl.c
+++ b/vl.c
@@ -1168,6 +1168,7 @@ static void default_drive(int enable, int snapshot, 
BlockInterfaceType type,
   int index, const char *optstr)
 {
 QemuOpts *opts;
+DriveInfo *dinfo;
 
 if (!enable || drive_get_by_index(type, index)) {
 return;
@@ -1177,9 +1178,13 @@ static void default_drive(int enable, int snapshot, 
BlockInterfaceType type,
 if (snapshot) {
 drive_enable_snapshot(opts, NULL);
 }
-if (!drive_new(opts, type)) {
+
+dinfo = drive_new(opts, type);
+if (!dinfo) {
 exit(1);
 }
+dinfo->is_default = true;
+
 }
 
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
@@ -4458,6 +4463,9 @@ int main(int argc, char **argv, char **envp)
 if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) 
!= 0)
 exit(1);
 
+/* Did we create any drives that we failed to create a device for? */
+drive_check_orphaned();
+
 net_check_clients();
 
 ds = init_displaystate();
-- 
1.9.3

[Qemu-devel] [PATCH v3 5/6] qtest/bios-tables: Correct Q35 command line

[John Snow]

If the Q35 board types are to begin recognizing
and decoding syntactic sugar for drive/device
declarations, then workarounds found within
the qtests suite need to be adjusted to prevent
any test failures after the fix.

bios-tables-test improperly uses this cli:
-drive file=etc,id=hd -device ide-hd,drive=hd

Which will create a drive and device due to
the lack of specifying if=none. Then, it will
attempt to create a second device and fail.

This patch corrects this test to always use
the full, non-sugared -device/-drive syntax
for both PC and Q35.

Signed-off-by: John Snow 
---
 tests/bios-tables-test.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 602932b..9e4d205 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -714,14 +714,12 @@ static void test_acpi_one(const char *params, test_data 
*data)
 uint8_t signature_high;
 uint16_t signature;
 int i;
-const char *device = "";
 
-if (!g_strcmp0(data->machine, MACHINE_Q35)) {
-device = ",id=hd -device ide-hd,drive=hd";
-}
+args = g_strdup_printf("-net none -display none %s "
+   "-drive id=hd0,if=none,file=%s "
+   "-device ide-hd,drive=hd0 ",
+   params ? params : "", disk);
 
-args = g_strdup_printf("-net none -display none %s -drive file=%s%s,",
-   params ? params : "", disk, device);
 qtest_start(args);
 
/* Wait at most 1 minute */
-- 
1.9.3

[Qemu-devel] [PATCH v3 2/6] blockdev: Allow overriding if_max_dev property

[John Snow]

The if_max_devs table as in the past been an immutable
default that controls the mapping of index => (bus,unit)
for all boards and all HBAs for each interface type.

Since adding this mapping information to the HBA device
itself is currently unwieldly from the perspective of
retrieving this information at option parsing time
(e.g, within drive_new), we consider the alternative
of marking the if_max_devs table mutable so that
later configuration and initialization can adjust the
mapping at will, but only up until a drive is added,
at which point the mapping is finalized.

Signed-off-by: John Snow 
---
 blockdev.c| 26 +-
 include/sysemu/blockdev.h |  2 ++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index d2ad065..1d9ab7f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -60,7 +60,7 @@ static const char *const if_name[IF_COUNT] = {
 [IF_XEN] = "xen",
 };
 
-static const int if_max_devs[IF_COUNT] = {
+static int if_max_devs[IF_COUNT] = {
 /*
  * Do not change these numbers!  They govern how drive option
  * index maps to unit and bus.  That mapping is ABI.
@@ -79,6 +79,30 @@ static const int if_max_devs[IF_COUNT] = {
 [IF_SCSI] = 7,
 };
 
+/**
+ * Boards may call this to offer board-by-board overrides
+ * of the default, global values.
+ */
+void override_max_devs(BlockInterfaceType type, int max_devs)
+{
+DriveInfo *dinfo;
+
+if (max_devs <= 0) {
+return;
+}
+
+QTAILQ_FOREACH(dinfo, &drives, next) {
+if (dinfo->type == type) {
+fprintf(stderr, "Cannot override units-per-bus property of"
+" the %s interface, because a drive of that type has"
+" already been added.\n", if_name[type]);
+g_assert_not_reached();
+}
+}
+
+if_max_devs[type] = max_devs;
+}
+
 /*
  * We automatically delete the drive when a device using it gets
  * unplugged.  Questionable feature, but we can't just drop it.
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 3040286..a4033d4 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -46,6 +46,8 @@ struct DriveInfo {
 QTAILQ_ENTRY(DriveInfo) next;
 };
 
+void override_max_devs(BlockInterfaceType type, int max_devs);
+
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
 bool drive_check_orphaned(void);
 DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
-- 
1.9.3

[Qemu-devel] [PATCH v3 3/6] pc/vl: Add units-per-default-bus property

[John Snow]

This patch adds the 'units_per_default_bus' property which
allows individual boards to declare their desired
index => (bus,unit) mapping for their default HBA, so that
boards such as Q35 can specify that its default if_ide HBA,
AHCI, only accepts one unit per bus.

This property only overrides the mapping for drives matching
the block_default_type interface.

This patch also adds this property to *all* past and present
Q35 machine types. This retroactive addition is justified
because the previous erroneous index=>(bus,unit) mappings
caused by lack of such a property were not utilized due to
lack of initialization code in the Q35 init routine.

Further, semantically, the Q35 board type has always had the
property that its default HBA, AHCI, only accepts one unit per
bus. The new code added to add devices to drives relies upon
the accuracy of this mapping. Thus, the property is applied
retroactively to reduce complexity of allowing IDE HBAs with
different units per bus.

Examples:

Prior to this patch, all IDE HBAs were assumed to use 2 units
per bus (Master, Slave). When using Q35 and AHCI, however, we
only allow one unit per bus.

-hdb foo.qcow2 would become index=1, or bus=0,unit=1.
-hdd foo.qcow2 would become index=3, or bus=1,unit=1.
-drive file=foo.qcow2,index=5 becomes bus=2,unit=1.

These are invalid for AHCI. They now become, under Q35 only:

-hdb foo.qcow2 --> index=1, bus=1, unit=0.
-hdd foo.qcow2 --> index=3, bus=3, unit=0.
-drive file=foo.qcow2,index=5 --> bus=5,unit=0.

The mapping is adjusted based on the fact that the default IF
for the Q35 machine type is IF_IDE, and units-per-default-bus
overrides the IDE mapping from its default of 2 units per bus
to just 1 unit per bus.

Signed-off-by: John Snow 
---
 hw/i386/pc.c| 1 +
 hw/i386/pc_q35.c| 3 ++-
 include/hw/boards.h | 2 ++
 vl.c| 8 
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 82a7daa..d045e8b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1524,6 +1524,7 @@ static void pc_generic_machine_class_init(ObjectClass 
*oc, void *data)
 mc->hot_add_cpu = qm->hot_add_cpu;
 mc->kvm_type = qm->kvm_type;
 mc->block_default_type = qm->block_default_type;
+mc->units_per_default_bus = qm->units_per_default_bus;
 mc->max_cpus = qm->max_cpus;
 mc->no_serial = qm->no_serial;
 mc->no_parallel = qm->no_parallel;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d4a907c..b28ddbb 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -344,7 +344,8 @@ static void pc_q35_init_1_4(MachineState *machine)
 #define PC_Q35_MACHINE_OPTIONS \
 PC_DEFAULT_MACHINE_OPTIONS, \
 .desc = "Standard PC (Q35 + ICH9, 2009)", \
-.hot_add_cpu = pc_hot_add_cpu
+.hot_add_cpu = pc_hot_add_cpu, \
+.units_per_default_bus = 1
 
 #define PC_Q35_2_2_MACHINE_OPTIONS  \
 PC_Q35_MACHINE_OPTIONS, \
diff --git a/include/hw/boards.h b/include/hw/boards.h
index dfb6718..663f16a 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -28,6 +28,7 @@ struct QEMUMachine {
 QEMUMachineHotAddCPUFunc *hot_add_cpu;
 QEMUMachineGetKvmtypeFunc *kvm_type;
 BlockInterfaceType block_default_type;
+int units_per_default_bus;
 int max_cpus;
 unsigned int no_serial:1,
 no_parallel:1,
@@ -86,6 +87,7 @@ struct MachineClass {
 int (*kvm_type)(const char *arg);
 
 BlockInterfaceType block_default_type;
+int units_per_default_bus;
 int max_cpus;
 unsigned int no_serial:1,
 no_parallel:1,
diff --git a/vl.c b/vl.c
index 6500472..940b149 100644
--- a/vl.c
+++ b/vl.c
@@ -1588,6 +1588,7 @@ static void machine_class_init(ObjectClass *oc, void 
*data)
 mc->hot_add_cpu = qm->hot_add_cpu;
 mc->kvm_type = qm->kvm_type;
 mc->block_default_type = qm->block_default_type;
+mc->units_per_default_bus = qm->units_per_default_bus;
 mc->max_cpus = qm->max_cpus;
 mc->no_serial = qm->no_serial;
 mc->no_parallel = qm->no_parallel;
@@ -4378,6 +4379,13 @@ int main(int argc, char **argv, char **envp)
 blk_mig_init();
 ram_mig_init();
 
+/* If the currently selected machine wishes to override the units-per-bus
+ * property of its default HBA interface type, do so now. */
+if (machine_class->units_per_default_bus) {
+override_max_devs(machine_class->block_default_type,
+  machine_class->units_per_default_bus);
+}
+
 /* open the virtual block devices */
 if (snapshot)
 qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, 
NULL, 0);
-- 
1.9.3

[Qemu-devel] [PATCH v3 4/6] ide: Update ide_drive_get to be HBA agnostic

[John Snow]

Instead of duplicating the logic for the if_ide
(bus,unit) mappings, rely on the blockdev layer
for managing those mappings for us, and use the
drive_get_by_index call instead.

This allows ide_drive_get to work for AHCI HBAs
as well, and can be used in the Q35 initialization.

Lastly, change the nature of the argument to
ide_drive_get so that represents the number of
total drives we can support, and not the total
number of buses. This will prevent array overflows
if the units-per-default-bus property ever needs
to be adjusted for compatibility reasons.

Signed-off-by: John Snow 
---
 blockdev.c| 17 +
 hw/alpha/dp264.c  |  2 +-
 hw/i386/pc_piix.c |  2 +-
 hw/ide/core.c | 22 +-
 hw/mips/mips_fulong2e.c   |  2 +-
 hw/mips/mips_malta.c  |  2 +-
 hw/mips/mips_r4k.c|  2 +-
 hw/ppc/mac_newworld.c |  2 +-
 hw/ppc/mac_oldworld.c |  2 +-
 hw/ppc/prep.c |  2 +-
 hw/sparc64/sun4u.c|  2 +-
 include/sysemu/blockdev.h |  1 +
 12 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1d9ab7f..4489090 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -135,6 +135,23 @@ void blockdev_auto_del(BlockDriverState *bs)
 }
 }
 
+/**
+ * Returns the current mapping of how many units per bus
+ * a particular interface can support.
+ *
+ *  A positive integer indicates n units per bus.
+ *  0 implies the mapping has not been established.
+ * -1 indicates an invalid BlockInterfaceType was given.
+ */
+int drive_get_max_devs(BlockInterfaceType type)
+{
+if (type >= IF_IDE && type < IF_COUNT) {
+return if_max_devs[type];
+}
+
+return -1;
+}
+
 static int drive_index_to_bus_id(BlockInterfaceType type, int index)
 {
 int max_devs = if_max_devs[type];
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index b178a03..84a55e4 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -97,7 +97,7 @@ static void clipper_init(MachineState *machine)
 /* IDE disk setup.  */
 {
 DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-ide_drive_get(hd, MAX_IDE_BUS);
+ide_drive_get(hd, ARRAY_SIZE(hd));
 
 pci_cmd646_ide_init(pci_bus, hd, 0);
 }
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 103d756..4384633 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -239,7 +239,7 @@ static void pc_init1(MachineState *machine,
 
 pc_nic_init(isa_bus, pci_bus);
 
-ide_drive_get(hd, MAX_IDE_BUS);
+ide_drive_get(hd, ARRAY_SIZE(hd));
 if (pci_enabled) {
 PCIDevice *dev;
 if (xen_enabled()) {
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 190700a..ae85428 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2558,16 +2558,28 @@ const VMStateDescription vmstate_ide_bus = {
 }
 };
 
-void ide_drive_get(DriveInfo **hd, int max_bus)
+void ide_drive_get(DriveInfo **hd, int n)
 {
 int i;
+int highest_bus = drive_get_max_bus(IF_IDE) + 1;
+int max_devs = drive_get_max_devs(IF_IDE);
+int n_buses = max_devs ? (n / max_devs) : n;
 
-if (drive_get_max_bus(IF_IDE) >= max_bus) {
-fprintf(stderr, "qemu: too many IDE bus: %d\n", max_bus);
+/*
+ * Note: The number of actual buses available is not known.
+ * We compute this based on the size of the DriveInfo* array, n.
+ * If it is less than max_devs * ,
+ * We will stop looking for drives prematurely instead of overfilling
+ * the array.
+ */
+
+if (highest_bus > n_buses) {
+error_report("Too many IDE buses defined (%d > %d)",
+ highest_bus, n_buses);
 exit(1);
 }
 
-for(i = 0; i < max_bus * MAX_IDE_DEVS; i++) {
-hd[i] = drive_get(IF_IDE, i / MAX_IDE_DEVS, i % MAX_IDE_DEVS);
+for (i = 0; i < n; i++) {
+hd[i] = drive_get_by_index(IF_IDE, i);
 }
 }
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index be286da..29cd708 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -350,7 +350,7 @@ static void mips_fulong2e_init(MachineState *machine)
 pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
 
 /* South bridge */
-ide_drive_get(hd, MAX_IDE_BUS);
+ide_drive_get(hd, ARRAY_SIZE(hd));
 
 isa_bus = vt82c686b_init(pci_bus, PCI_DEVFN(FULONG2E_VIA_SLOT, 0));
 if (!isa_bus) {
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 2d87de9..b20807c 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1147,7 +1147,7 @@ void mips_malta_init(MachineState *machine)
 pci_bus = gt64120_register(isa_irq);
 
 /* Southbridge */
-ide_drive_get(hd, MAX_IDE_BUS);
+ide_drive_get(hd, ARRAY_SIZE(hd));
 
 piix4_devfn = piix4_init(pci_bus, &isa_bus, 80);
 
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index e219766..93606a4 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -294,7 +294,7 @@ void mips_r4k_init(MachineState *machine)
 if (nd_table[0].used)
 isa

[Qemu-devel] [PATCH v3 6/6] q35/ahci: Pick up -cdrom and -hda options

[John Snow]

This patch implements the backend for the Q35 board
for us to be able to pick up and use drives defined
by the -cdrom, -hda, or -drive if=ide shorthand options.

Signed-off-by: John Snow 
---
 hw/i386/pc_q35.c |  4 
 hw/ide/ahci.c| 15 +++
 hw/ide/ahci.h|  2 ++
 3 files changed, 21 insertions(+)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b28ddbb..bb0dc8e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -86,6 +86,7 @@ static void pc_q35_init(MachineState *machine)
 DeviceState *icc_bridge;
 PcGuestInfo *guest_info;
 ram_addr_t lowmem;
+DriveInfo *hd[MAX_SATA_PORTS];
 
 /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
  * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
@@ -253,6 +254,9 @@ static void pc_q35_init(MachineState *machine)
true, "ich9-ahci");
 idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0");
 idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1");
+g_assert_cmpint(MAX_SATA_PORTS, ==, ICH_AHCI(ahci)->ahci.ports);
+ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports);
+ahci_ide_create_devs(ahci, hd);
 
 if (usb_enabled(false)) {
 /* Should we create 6 UHCI according to ich9 spec? */
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 8978643..063730e 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1419,3 +1419,18 @@ static void sysbus_ahci_register_types(void)
 }
 
 type_init(sysbus_ahci_register_types)
+
+void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd)
+{
+AHCIPCIState *d = ICH_AHCI(dev);
+AHCIState *ahci = &d->ahci;
+int i;
+
+for (i = 0; i < ahci->ports; i++) {
+if (hd[i] == NULL) {
+continue;
+}
+ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
+}
+
+}
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 1543df7..e223258 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -332,4 +332,6 @@ void ahci_uninit(AHCIState *s);
 
 void ahci_reset(AHCIState *s);
 
+void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
+
 #endif /* HW_IDE_AHCI_H */
-- 
1.9.3

[Qemu-devel] [PATCH v3 0/6] Q35: Implement -cdrom/-hda sugar

[John Snow]

The Q35 board initialization does not currently bother to look
for any drives added by the various syntactical sugar shorthands
to be added to the AHCI HBA. These include -hda through -hdd,
-cdrom, and -drive if=ide shorthands.

An obstacle to having implemented this sooner is debate over
whether or not to add an additional interface type, and how to
manage the different units-per-bus mappings of various HBA
implementations.

This patch series:
(1) Does not add IF_AHCI, but reuses IF_IDE
(2) Allows the if_max_devs table to be overridden
(3) Adds this override to the Q35 board type.
(4) Finally, adds implementation to Q35 initialization.

History:

V3:

- renamed a variable for consistency ("tab" to "hd")
- Now uses ARRAY_SIZE() for calls to ide_drive_get where applicable
- Re-added a call to exit() for the error pathway of ide_drive_get
- Adjusted drive_get_max_devs semantics to be similar to drive_get_max_bus
  and added documentation
- Removed any possibility of a divide-by-zero in ide_drive_get

V2:
- Adjusted language in patch #1's commit message.
  (drive if=none will NOT trigger warnings)
- Removed superfluous warning with bad phrasing in patch #1
- Removed if_get_max_devs from patch #2 and added to patch #4
- Added an assertion to patch #2
- Added more detail to patch #3's commit message
- Specified that Patch #3 will affect old Q35 machine types
- Changed fprintf to error_report in patch #4
- Replaced max_bus parameter in ide_drive_get with 'n', size of array
- Updated calls to ide_drive_get in other boards
- Adjusted language in patch #6's commit message.
  (Removed reference to patch #5.)

V1:
- Re-uses ide_drive_get instead of ahci_drive_get
- Adds units-per-bus property to all Q35 machines
- Changes orphan scanning to exclude IF_NONE and
  automatically added drives
- Renames 'units-per-idebus' to 'units-per-default-bus'
  And allows override of any one IF type (block_default)

RFC2:
- Rewrote series to avoid the creation of IF_AHCI.

John Snow (6):
  blockdev: Orphaned drive search
  blockdev: Allow overriding if_max_dev property
  pc/vl: Add units-per-default-bus property
  ide: Update ide_drive_get to be HBA agnostic
  qtest/bios-tables: Correct Q35 command line
  q35/ahci: Pick up -cdrom and -hda options

 blockdev.c| 64 ++-
 hw/alpha/dp264.c  |  2 +-
 hw/i386/pc.c  |  1 +
 hw/i386/pc_piix.c |  2 +-
 hw/i386/pc_q35.c  |  7 +-
 hw/ide/ahci.c | 15 +++
 hw/ide/ahci.h |  2 ++
 hw/ide/core.c | 22 
 hw/mips/mips_fulong2e.c   |  2 +-
 hw/mips/mips_malta.c  |  2 +-
 hw/mips/mips_r4k.c|  2 +-
 hw/ppc/mac_newworld.c |  2 +-
 hw/ppc/mac_oldworld.c |  2 +-
 hw/ppc/prep.c |  2 +-
 hw/sparc64/sun4u.c|  2 +-
 include/hw/boards.h   |  2 ++
 include/sysemu/blockdev.h |  5 
 tests/bios-tables-test.c  | 10 +++-
 vl.c  | 18 -
 19 files changed, 141 insertions(+), 23 deletions(-)

-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 4/7] tests: Add unit test for X86CPU code

[Paolo Bonzini]

Il 01/10/2014 18:28, Eduardo Habkost ha scritto:
>> > tests/x86-stub.c perhaps can be moved to target-i386/test-stubs.c?
> I was trying to keep all test code inside tests/. But perhaps all the
> target-specific test code (including test-x86-cpu.c) could be moved to
> target directories, and we could build/run the target-specific test
> cases from Makefile.target. That should simplify some of the logic I
> have added, and fix the build dependency problem I mentioned in the
> patch description.

In the end that's just bikeshedding.  Sooner or later we'll have to
organize tests/ in subdirectories, but it's probably early enough that
we may not care yet.  tests/x86-stub.c be it.

Paolo

Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets

[Paolo Bonzini]

Il 01/10/2014 14:38, Corey Minyard ha scritto:
> I haven't heard anything about these patches.  Is there anything I need
> to do to get them included?

Nothing, I just missed them in the huge traffic of qemu-devel.  Will
look at them tomorrow.

Thanks for pinging them.

Paolo

Re: [Qemu-devel] [PATCH] qemu-file: Add copyright header to qemu-file.c

[Dr. David Alan Gilbert]

* Eduardo Habkost (ehabk...@redhat.com) wrote:
> The person who created qemu-file.c (me, on commit
> 093c455a8c6d8f715eabd8c8d346f08f17d686ec) didn't add a copyright/license
> header to the file, even though the whole code was copied from savevm.c
> (which had a copyright/license header).
> 
> To correct this, copy the copyright information and license from
> savevm.c, that's where the original code came from.
> 
> Luckily, very few changes were made on qemu-file.c after it was created.
> All the authors who touched the code are being CCed, so they can confirm
> if they are OK with the copyright/license information being added.

Since it's come from vl.c that seems to make sense.
It should probably go to the stable branch as well, since reading the ~/LICENSE
file would make you think it's GPLd.

Reviewed-by: Dr. David Alan Gilbert 

> Cc: Dr. David Alan Gilbert 
> Cc: Alexey Kardashevskiy 
> Cc: Markus Armbruster 
> Cc: Juan Quintela 
> Signed-off-by: Eduardo Habkost 
> ---
>  qemu-file.c | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/qemu-file.c b/qemu-file.c
> index a8e3912..bd5d4af 100644
> --- a/qemu-file.c
> +++ b/qemu-file.c
> @@ -1,3 +1,26 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
>  #include "qemu-common.h"
>  #include "qemu/iov.h"
>  #include "qemu/sockets.h"
> -- 
> 1.9.3
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2] qga: Rewrite code where using readdir_r

[Michael Roth]

Quoting zhanghailiang (2014-09-18 22:09:10)
> If readdir_r fails, error_setg_errno will reference the freed
> pointer *dirpath*.
> 
> Moreover, readdir_r may cause a buffer overflow, using readdir instead.
> 
> Signed-off-by: zhanghailiang 

Thanks, applied to qga tree:

https://github.com/mdroth/qemu/commits/qga

> ---
>  v2:
> - Switch readdir_r to readdir (Comment of Eric Blake)
> ---
>  qga/commands-posix.c | 27 +++
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 7eed7f4..f6f3e3c 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -956,7 +956,7 @@ static void build_guest_fsinfo_for_virtual_device(char 
> const *syspath,
>  {
>  DIR *dir;
>  char *dirpath;
> -struct dirent entry, *result;
> +struct dirent *entry;
> 
>  dirpath = g_strdup_printf("%s/slaves", syspath);
>  dir = opendir(dirpath);
> @@ -965,22 +965,24 @@ static void build_guest_fsinfo_for_virtual_device(char 
> const *syspath,
>  g_free(dirpath);
>  return;
>  }
> -g_free(dirpath);
> 
>  for (;;) {
> -if (readdir_r(dir, &entry, &result) != 0) {
> -error_setg_errno(errp, errno, "readdir_r(\"%s\")", dirpath);
> -break;
> -}
> -if (!result) {
> +errno = 0;
> +entry = readdir(dir);
> +if (entry == NULL) {
> +if (errno) {
> +error_setg_errno(errp, errno, "readdir(\"%s\")", dirpath);
> +}
>  break;
>  }
> 
> -if (entry.d_type == DT_LNK) {
> -g_debug(" slave device '%s'", entry.d_name);
> -dirpath = g_strdup_printf("%s/slaves/%s", syspath, entry.d_name);
> -build_guest_fsinfo_for_device(dirpath, fs, errp);
> -g_free(dirpath);
> +if (entry->d_type == DT_LNK) {
> +char *path;
> +
> +g_debug(" slave device '%s'", entry->d_name);
> +path = g_strdup_printf("%s/slaves/%s", syspath, entry->d_name);
> +build_guest_fsinfo_for_device(path, fs, errp);
> +g_free(path);
> 
>  if (*errp) {
>  break;
> @@ -988,6 +990,7 @@ static void build_guest_fsinfo_for_virtual_device(char 
> const *syspath,
>  }
>  }
> 
> +g_free(dirpath);
>  closedir(dir);
>  }
> 
> -- 
> 1.7.12.4

[Qemu-devel] [PATCH] qemu-file: Add copyright header to qemu-file.c

[Eduardo Habkost]

The person who created qemu-file.c (me, on commit
093c455a8c6d8f715eabd8c8d346f08f17d686ec) didn't add a copyright/license
header to the file, even though the whole code was copied from savevm.c
(which had a copyright/license header).

To correct this, copy the copyright information and license from
savevm.c, that's where the original code came from.

Luckily, very few changes were made on qemu-file.c after it was created.
All the authors who touched the code are being CCed, so they can confirm
if they are OK with the copyright/license information being added.

Cc: Dr. David Alan Gilbert 
Cc: Alexey Kardashevskiy 
Cc: Markus Armbruster 
Cc: Juan Quintela 
Signed-off-by: Eduardo Habkost 
---
 qemu-file.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/qemu-file.c b/qemu-file.c
index a8e3912..bd5d4af 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -1,3 +1,26 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
 #include "qemu-common.h"
 #include "qemu/iov.h"
 #include "qemu/sockets.h"
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2] qemu-ga: added windows support for 'guest-network-get-interfaces'

[Michael Roth]

Quoting Kenth Andersson (2014-09-29 13:22:54)
> Implementation of guest-network-get-interfaces for windows
> 
> 
> Signed-off-by: Kenth Andersson 

Thanks! I've been testing the functionality and it seems work nicely. Some
review comments below though:

> ---
>  configure|   2 +-
>  qga/commands-win32.c | 247 
> ++-
>  2 files changed, 244 insertions(+), 5 deletions(-)
> 
> diff --git a/configure b/configure
> index 681abfc..7c6c60c 100755
> --- a/configure
> +++ b/configure
> @@ -717,7 +717,7 @@ EOF
>sysconfdir="\${prefix}"
>local_statedir=
>confsuffix=""
> -  libs_qga="-lws2_32 -lwinmm -lpowrprof $libs_qga"
> +  libs_qga="-lws2_32 -lwinmm -lpowrprof -liphlpapi $libs_qga"
>  fi
>  
>  werror=""
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 3bcbeae..fb6fdba 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -14,6 +14,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include "qga/guest-agent-core.h"
>  #include "qga/vss-win32.h"
>  #include "qga-qmp-commands.h"
> @@ -29,6 +32,9 @@
> (365 * (1970 - 1601) +   \
>  (1970 - 1601) / 4 - 3))
>  
> +/* Defines the max length of an IPv6 address in human readable format + pad 
> */
> +#define MAX_IPv6_LEN 50
> +
>  static void acquire_privilege(const char *name, Error **errp)
>  {
>  HANDLE token = NULL;
> @@ -359,10 +365,244 @@ void qmp_guest_suspend_hybrid(Error **errp)
>  error_set(errp, QERR_UNSUPPORTED);
>  }
>  
> +static int get_prefix_length(PIP_ADAPTER_PREFIX prefix,
> +  SOCKET_ADDRESS *sockaddr, int socklen)
> +{
> +PIP_ADAPTER_PREFIX p;
> +struct sockaddr *addr = sockaddr->lpSockaddr;
> +
> +for (p = prefix; p; p = p->Next) {
> +/* Compare the ip-adderss address family with the prefix
> + * address family */
> +if (addr->sa_family == p->Address.lpSockaddr->sa_family) {
> +int num_bytes = p->PrefixLength / 8;
> +unsigned char *src = 0;
> +unsigned char *dst = 0;
> +int remaining_bits;
> +
> +if (addr->sa_family == AF_INET) {
> +struct sockaddr_in *sin = (struct sockaddr_in *)addr;
> +src = (unsigned char *)&(sin->sin_addr.s_addr);
> +} else if (addr->sa_family == AF_INET6) {
> +struct sockaddr_in6 *sin = (struct sockaddr_in6 *)addr;
> +src = (unsigned char *)sin->sin6_addr.s6_addr;
> +}
> +
> +if (p->Address.lpSockaddr->sa_family == AF_INET) {
> +struct sockaddr_in *sin =
> +(struct sockaddr_in *)p->Address.lpSockaddr;
> +dst = (unsigned char *)&(sin->sin_addr.s_addr);
> +} else if (p->Address.lpSockaddr->sa_family == AF_INET6) {
> +struct sockaddr_in6 *sin =
> +(struct sockaddr_in6 *)p->Address.lpSockaddr;
> +dst = (unsigned char *)sin->sin6_addr.s6_addr;
> +}
> +
> +/* If non of the addresses are in correct format we will continue
> + * to next one */
> +if (src == 0 || dst == 0) {
> +continue;
> +}
> +
> +/* Check if prefix network is the same network as we are testing
> + * start with whole bytes */
> +
> +if (memcmp(src, dst, num_bytes) != 0) {
> +continue;
> +}
> +
> +/* Check the remaning bits */
> +remaining_bits = p->PrefixLength % 8;
> +
> +if (remaining_bits != 0) {
> +unsigned char mask = 0xff << (8 - remaining_bits);
> +int i = num_bytes;
> +
> +if ((src[i] & mask) != (dst[i] & mask)) {
> +continue;
> +}
> +}
> +
> +return p->PrefixLength;
> +}
> +}
> +return 0;
> +}
> +
> +
> +
>  GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>  {
> -error_set(errp, QERR_UNSUPPORTED);
> -return NULL;
> +GuestNetworkInterfaceList *head = NULL, *curr_item = NULL;
> +DWORD ret_val;
> +
> +PIP_ADAPTER_ADDRESSES adpt_addrs;
> +PIP_ADAPTER_ADDRESSES curr_adpt_addrs;
> +PIP_ADAPTER_UNICAST_ADDRESS adpt_uc_addr;
> +
> +/* GetAdaptersAddresses requires ULONG */
> +ULONG bufLen = sizeof(IP_ADAPTER_ADDRESSES);

buf_len

> +
> +/* Startup WinSock32 */
> +WORD ws_version_requested = MAKEWORD(2, 2);
> +WSADATA ws_data;
> +WSAStartup(ws_version_requested, &ws_data);

Since this can fail, we should return an error and bail (after calling 
WSACleanup).
Something like:

  error_setg(errp, "failed to initialize Windows Socket API v2.2");

> +
> +/* Allocate adapter address list with one entry, used to
> + * fetch the read length needed by GetAdapterA

Re: [Qemu-devel] [RFC patch 0/6] vfio based pci pass-through for qemu/KVM on s390

[Alex Williamson]

On Wed, 2014-10-01 at 11:11 +0200, Frank Blaschka wrote:
> On Fri, Sep 26, 2014 at 01:59:40PM -0600, Alex Williamson wrote:
> > On Fri, 2014-09-26 at 08:45 +0200, Frank Blaschka wrote:
> > > On Wed, Sep 24, 2014 at 10:05:57AM -0600, Alex Williamson wrote:
> > > > On Wed, 2014-09-24 at 10:47 +0200, Frank Blaschka wrote:
> > > > > On Mon, Sep 22, 2014 at 02:47:31PM -0600, Alex Williamson wrote:
> > > > > > On Fri, 2014-09-19 at 13:54 +0200, frank.blasc...@de.ibm.com wrote:
> > > > > > > This set of patches implements a vfio based solution for pci
> > > > > > > pass-through on the s390 platform. The kernel stuff is pretty
> > > > > > > much straight forward, but qemu needs more work.
> > > > > > > 
> > > > > > > Most interesting patch is:
> > > > > > >   vfio: make vfio run on s390 platform
> > > > > > > 
> > > > > > > I hope Alex & Alex can give me some guidance how to do the changes
> > > > > > > in an appropriate way. After creating a separate iommmu address 
> > > > > > > space
> > > > > > > for each attached PCI device I can successfully run the vfio type1
> > > > > > > iommu. So If we could extend type1 not registering all guest 
> > > > > > > memory
> > > > > > > (see patch) I think we do not need a special vfio iommu for s390
> > > > > > > for the moment.
> > > > > > > 
> > > > > > > The patches implement the base pass-through support. s390 specific
> > > > > > > virtualization functions are currently not included. This would
> > > > > > > be a second step after the base support is done.
> > > > > > > 
> > > > > > > kernel patches apply to linux-kvm-next
> > > > > > > 
> > > > > > > KVM: s390: Enable PCI instructions
> > > > > > > iommu: add iommu for s390 platform
> > > > > > > vfio: make vfio build on s390
> > > > > > > 
> > > > > > > qemu patches apply to qemu-master
> > > > > > > 
> > > > > > > s390: Add PCI bus support
> > > > > > > s390: implement pci instruction
> > > > > > > vfio: make vfio run on s390 platform
> > > > > > > 
> > > > > > > Thx for feedback and review comments
> > > > > > 
> > > > > > Sending patches as attachments makes it difficult to comment inline.
> > > > > >
> > > > > Sorry, don't understand this. I sent every patch as separate email so
> > > > > you can comment directly on the patch. What do you prefer?
> > > > 
> > > > The patches in each email are showing up as attachments in my mail
> > > > client.  Is it just me?
> > > > 
> > > > > > 2/6
> > > > > >  - careful of the namespace as you're changing functions from 
> > > > > > static and
> > > > > > exporting them
> > > > > >  - doesn't seem like functions need to be exported, just non-static 
> > > > > > to
> > > > > > call from s390-iommu.c
> > > > > > 
> > > > > Ok, will change this.
> > > > > 
> > > > > > 6/6
> > > > > >  - We shouldn't need to globally disable mmap, each VFIO region 
> > > > > > reports
> > > > > > whether it supports mmap and vfio-pci on s390 should indicate mmap 
> > > > > > is
> > > > > > not supported on the platform.
> > > > > Yes, this is even better to let the kernel announce a BAR can not be
> > > > > mmap'ed. Checking the kernel code I realized the BARs are valid for
> > > > > mmap'ing but the s390 platform does simply not allow this. So I feal 
> > > > > we
> > > > > have to introduce a platform switch in kernel. How about this ...
> > > > > 
> > > > > --- a/drivers/vfio/pci/vfio_pci.c
> > > > > +++ b/drivers/vfio/pci/vfio_pci.c
> > > > > @@ -377,9 +377,11 @@ static long vfio_pci_ioctl(void *device_
> > > > > 
> > > > > info.flags = VFIO_REGION_INFO_FLAG_READ |
> > > > >  VFIO_REGION_INFO_FLAG_WRITE;
> > > > > +#ifndef CONFIG_S390
> > > > > if (pci_resource_flags(pdev, info.index) &
> > > > > IORESOURCE_MEM && info.size >= PAGE_SIZE)
> > > > > info.flags |= 
> > > > > VFIO_REGION_INFO_FLAG_MMAP;
> > > > > +#endif
> > > > > break;
> > > > > case VFIO_PCI_ROM_REGION_INDEX:
> > > > > {
> > > > 
> > > > Maybe pull it out into a function.  Also, is there some capability or
> > > > feature we can test rather than just the architecture?  I'd prefer it to
> > > > be excluded because of a platform feature that prevents it rather than
> > > > the overall architecture itself.
> > > >
> > > 
> > > Ok, understand this. There is no capability of feature so I will go with
> > > the function.
> > >  
> > > > > >  - INTx should be done the same way, the interrupt index for INTx 
> > > > > > should
> > > > > > report 0 count.  The current code likely doesn't handle this, but it
> > > > > > should be easy to fix.
> > > > > The current code is fine. Problem is the card reports an interrupt 
> > > > > index
> > > > > (PCI_INTERRUPT_PIN) but again the platform does not support INTx at 
> > > > > all.
> > > > > So we need a platform switch as well. 
> > > > 
> > > > Yep, let's try to do something consistent with the

Re: [Qemu-devel] [PATCH v4 23/23] block: Make device model's references to BlockBackend strong

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

Doesn't make a difference just yet, but it's the right thing to do.

Signed-off-by: Markus Armbruster 
Reviewed-by: Benoît Canet 
Reviewed-by: Kevin Wolf 
---
  block/block-backend.c | 2 ++
  1 file changed, 2 insertions(+)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v4 19/23] blockdev: Fix blockdev-add not to create DriveInfo

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

blockdev_init() always creates a DriveInfo, but only drive_new() fills
it in.  qmp_blockdev_add() leaves it blank.  This results in a drive
with type = IF_IDE, bus = 0, unit = 0.  Screwed up in commit ee13ed1c.

Board initialization code looking for IDE drive (0,0) can pick up one
of these bogus drives.  The QMP command has to execute really early to
be visible.  Not sure how likely that is in practice.

Fix by creating DriveInfo in drive_new().  Block backends created by
blockdev-add don't get one.

Breaks the test for "has been created by qmp_blockdev_add()" in
blockdev_mark_auto_del() and do_drive_del(), because it changes the
value of dinfo && !dinfo->enable_auto_del from true to false.  Simply
test !dinfo instead.

Leaves DriveInfo member enable_auto_del unused.  Drop it.

A few places assume a block backend always has a DriveInfo.  Fix them
up.

Signed-off-by: Markus Armbruster 
---
  block/block-backend.c |  2 +-
  blockdev.c| 22 +++---
  hw/block/block.c  | 16 ++--
  hw/ide/qdev.c |  2 +-
  hw/scsi/scsi-disk.c   |  2 +-
  include/sysemu/blockdev.h |  1 -
  6 files changed, 20 insertions(+), 25 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v4 15/23] hw: Convert from BlockDriverState to BlockBackend, mostly

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

Device models should access their block backends only through the
block-backend.h API.  Convert them, and drop direct includes of
inappropriate headers.

Just four uses of BlockDriverState are left:

* The Xen paravirtual block device backend (xen_disk.c) opens images
   itself when set up via xenbus, bypassing blockdev.c.  I figure it
   should go through qmp_blockdev_add() instead.

* Device model "usb-storage" prompts for keys.  No other device model
   does, and this one probably shouldn't do it, either.

* ide_issue_trim_cb() uses bdrv_aio_discard() instead of
   blk_aio_discard() because it fishes its backend out of a BlockAIOCB,
   which has only the BlockDriverState.

* PC87312State has an unused BlockDriverState[] member.

The next two commits take care of the latter two.

Signed-off-by: Markus Armbruster 
---
  block/block-backend.c| 262 +++
  blockdev.c   |  19 +--
  dma-helpers.c|  59 +++
  hw/arm/collie.c  |   5 +-
  hw/arm/gumstix.c |   5 +-
  hw/arm/highbank.c|   2 +-
  hw/arm/mainstone.c   |   2 +-
  hw/arm/musicpal.c|  10 +-
  hw/arm/nseries.c |   3 +-
  hw/arm/omap1.c   |   2 +-
  hw/arm/omap2.c   |   2 +-
  hw/arm/omap_sx1.c|   5 +-
  hw/arm/pxa2xx.c  |   4 +-
  hw/arm/realview.c|   2 +-
  hw/arm/spitz.c   |   4 +-
  hw/arm/tosa.c|   3 +-
  hw/arm/versatilepb.c |   3 +-
  hw/arm/vexpress.c|   3 +-
  hw/arm/virt.c|   2 +-
  hw/arm/xilinx_zynq.c |   3 +-
  hw/arm/z2.c  |   3 +-
  hw/block/block.c |   7 +-
  hw/block/dataplane/virtio-blk.c  |  24 +--
  hw/block/fdc.c   |  78 +
  hw/block/hd-geometry.c   |  24 +--
  hw/block/m25p80.c|  28 ++--
  hw/block/nand.c  |  50 +++---
  hw/block/nvme.c  |  19 +--
  hw/block/onenand.c   |  67 
  hw/block/pflash_cfi01.c  |  24 +--
  hw/block/pflash_cfi02.c  |  24 +--
  hw/block/virtio-blk.c|  95 +--
  hw/block/xen_disk.c  |  83 +-
  hw/core/qdev-properties-system.c |  26 +--
  hw/core/qdev-properties.c|   2 +-
  hw/cris/axis_dev88.c |   3 +-
  hw/display/tc6393xb.c|   2 +-
  hw/i386/pc.c |   2 +-
  hw/i386/pc_piix.c|   2 +-
  hw/i386/pc_sysfw.c   |   9 +-
  hw/i386/xen/xen_platform.c   |   5 +-
  hw/ide/ahci.c|  31 ++--
  hw/ide/atapi.c   |  33 ++--
  hw/ide/cmd646.c  |   2 +-
  hw/ide/core.c| 184 --
  hw/ide/ich.c |   2 +-
  hw/ide/internal.h|   6 +-
  hw/ide/isa.c |   2 +-
  hw/ide/macio.c   |  50 +++---
  hw/ide/microdrive.c  |   4 +-
  hw/ide/mmio.c|   2 +-
  hw/ide/pci.c |   4 +-
  hw/ide/piix.c|   9 +-
  hw/ide/qdev.c|  11 +-
  hw/ide/via.c |   2 +-
  hw/isa/pc87312.c |   4 +-
  hw/lm32/lm32_boards.c|   5 +-
  hw/lm32/milkymist.c  |   3 +-
  hw/microblaze/petalogix_ml605_mmu.c  |   3 +-
  hw/microblaze/petalogix_s3adsp1800_mmu.c |   3 +-
  hw/mips/mips_fulong2e.c  |   2 +-
  hw/mips/mips_jazz.c  |   2 +-
  hw/mips/mips_malta.c |   6 +-
  hw/mips/mips_r4k.c   |   3 +-
  hw/nvram/spapr_nvram.c   |  17 +-
  hw/pci/pci-hotplug-old.c |   5 +-
  hw/ppc/mac_newworld.c|   2 +-
  hw/ppc/mac_oldworld.c|   2 +-
  hw/ppc/ppc405_boards.c   |  26 +--
  hw/ppc/prep.c|   2 +-
  hw/ppc/spapr.c   |   4 +-
  hw/ppc/virtex_ml507.c|   3 +-
  hw/s390x/s390-virtio-bus.c   |   2 +-
  hw/s390x/s390-virtio.c   |   2 +-
  hw/s390x/virtio-ccw.c|   2 +-
  hw/scsi/megasas.c|  15 +-
  hw/scsi/scsi-bus.c   |   8 +-
  hw/scsi/

[Qemu-devel] [PATCH 10/11] block: let commit blockjob run in BDS AioContext

[Stefan Hajnoczi]

The commit block job must run in the BlockDriverState AioContext so that
it works with dataplane.

Acquire the AioContext in blockdev.c so starting the block job is safe.
One detail here is that the bdrv_drain_all() must be moved inside the
aio_context_acquire() region so requests cannot sneak in between the
drain and acquire.

The completion code in block/commit.c must perform backing chain
manipulation and bdrv_reopen() from the main loop.  Use
block_job_defer_to_main_loop() to achieve that.

Signed-off-by: Stefan Hajnoczi 
---
 block/commit.c | 72 +-
 blockdev.c | 29 +++
 2 files changed, 66 insertions(+), 35 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index 91517d3..0fd05dc 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -60,17 +60,50 @@ static int coroutine_fn commit_populate(BlockDriverState 
*bs,
 return 0;
 }
 
-static void coroutine_fn commit_run(void *opaque)
+typedef struct {
+int ret;
+} CommitCompleteData;
+
+static void commit_complete(BlockJob *job, void *opaque)
 {
-CommitBlockJob *s = opaque;
+CommitBlockJob *s = container_of(job, CommitBlockJob, common);
+CommitCompleteData *data = opaque;
 BlockDriverState *active = s->active;
 BlockDriverState *top = s->top;
 BlockDriverState *base = s->base;
 BlockDriverState *overlay_bs;
+int ret = data->ret;
+
+if (!block_job_is_cancelled(&s->common) && ret == 0) {
+/* success */
+ret = bdrv_drop_intermediate(active, top, base, s->backing_file_str);
+}
+
+/* restore base open flags here if appropriate (e.g., change the base back
+ * to r/o). These reopens do not need to be atomic, since we won't abort
+ * even on failure here */
+if (s->base_flags != bdrv_get_flags(base)) {
+bdrv_reopen(base, s->base_flags, NULL);
+}
+overlay_bs = bdrv_find_overlay(active, top);
+if (overlay_bs && s->orig_overlay_flags != bdrv_get_flags(overlay_bs)) {
+bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
+}
+g_free(s->backing_file_str);
+block_job_completed(&s->common, ret);
+g_free(data);
+}
+
+static void coroutine_fn commit_run(void *opaque)
+{
+CommitBlockJob *s = opaque;
+CommitCompleteData *data;
+BlockDriverState *top = s->top;
+BlockDriverState *base = s->base;
 int64_t sector_num, end;
 int ret = 0;
 int n = 0;
-void *buf;
+void *buf = NULL;
 int bytes_written = 0;
 int64_t base_len;
 
@@ -78,18 +111,18 @@ static void coroutine_fn commit_run(void *opaque)
 
 
 if (s->common.len < 0) {
-goto exit_restore_reopen;
+goto out;
 }
 
 ret = base_len = bdrv_getlength(base);
 if (base_len < 0) {
-goto exit_restore_reopen;
+goto out;
 }
 
 if (base_len < s->common.len) {
 ret = bdrv_truncate(base, s->common.len);
 if (ret) {
-goto exit_restore_reopen;
+goto out;
 }
 }
 
@@ -128,7 +161,7 @@ wait:
 if (s->on_error == BLOCKDEV_ON_ERROR_STOP ||
 s->on_error == BLOCKDEV_ON_ERROR_REPORT||
 (s->on_error == BLOCKDEV_ON_ERROR_ENOSPC && ret == -ENOSPC)) {
-goto exit_free_buf;
+goto out;
 } else {
 n = 0;
 continue;
@@ -140,27 +173,14 @@ wait:
 
 ret = 0;
 
-if (!block_job_is_cancelled(&s->common) && sector_num == end) {
-/* success */
-ret = bdrv_drop_intermediate(active, top, base, s->backing_file_str);
+out:
+if (buf) {
+qemu_vfree(buf);
 }
 
-exit_free_buf:
-qemu_vfree(buf);
-
-exit_restore_reopen:
-/* restore base open flags here if appropriate (e.g., change the base back
- * to r/o). These reopens do not need to be atomic, since we won't abort
- * even on failure here */
-if (s->base_flags != bdrv_get_flags(base)) {
-bdrv_reopen(base, s->base_flags, NULL);
-}
-overlay_bs = bdrv_find_overlay(active, top);
-if (overlay_bs && s->orig_overlay_flags != bdrv_get_flags(overlay_bs)) {
-bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
-}
-g_free(s->backing_file_str);
-block_job_completed(&s->common, ret);
+data = g_malloc(sizeof(*data));
+data->ret = ret;
+block_job_defer_to_main_loop(&s->common, commit_complete, data);
 }
 
 static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp)
diff --git a/blockdev.c b/blockdev.c
index 5cf2058..d9333d7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1987,6 +1987,7 @@ void qmp_block_commit(const char *device,
 {
 BlockDriverState *bs;
 BlockDriverState *base_bs, *top_bs;
+AioContext *aio_context;
 Error *local_err = NULL;
 /* This will be part of the QMP command, if/when the
  * BlockdevOnError change for blkmirror makes it in
@@ -1997,9 +1998,6 @@ void qmp_block_commit(const char *device,
 speed = 

[Qemu-devel] [PATCH 11/11] block: declare blockjobs and dataplane friends!

[Stefan Hajnoczi]

Now that blockjobs use AioContext they are safe for use with dataplane.
Unblock them!

Signed-off-by: Stefan Hajnoczi 
---
 blockjob.c  | 1 +
 hw/block/dataplane/virtio-blk.c | 5 +
 2 files changed, 6 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 24a64d8..d0b753f 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -50,6 +50,7 @@ void *block_job_create(const BlockJobDriver *driver, 
BlockDriverState *bs,
 error_setg(&job->blocker, "block device is in use by block job: %s",
BlockJobType_lookup[driver->job_type]);
 bdrv_op_block_all(bs, job->blocker);
+bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
 
 job->driver= driver;
 job->bs= bs;
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 5458f9d..0603230 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -195,6 +195,11 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, 
VirtIOBlkConf *blk,
 bdrv_op_block_all(blk->conf.bs, s->blocker);
 bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_RESIZE, s->blocker);
 bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker);
+bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_BACKUP_SOURCE, s->blocker);
+bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_COMMIT, s->blocker);
+bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_MIRROR, s->blocker);
+bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_STREAM, s->blocker);
+bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_REPLACE, s->blocker);
 
 *dataplane = s;
 }
-- 
1.9.3

[Qemu-devel] [PATCH 09/11] block: let mirror blockjob run in BDS AioContext

[Stefan Hajnoczi]

The mirror block job must run in the BlockDriverState AioContext so that
it works with dataplane.

Acquire the AioContext in blockdev.c so starting the block job is safe.

Note that to_replace is treated separately from other BlockDriverStates
in that it does not need to be in the same AioContext.  Explicitly
acquire/release to_replace's AioContext when accessing it.

The completion code in block/mirror.c must perform BDS graph
manipulation and bdrv_reopen() from the main loop.  Use
block_job_defer_to_main_loop() to achieve that.

The bdrv_drain_all() call is not allowed outside the main loop since it
could lead to lock ordering problems.  Use bdrv_drain(bs) instead
because we have acquired the AioContext so nothing else can sneak in
I/O.

Signed-off-by: Stefan Hajnoczi 
---
 block.c| 13 +++--
 block/mirror.c | 85 --
 blockdev.c | 38 ++
 3 files changed, 97 insertions(+), 39 deletions(-)

diff --git a/block.c b/block.c
index 48305c4..9966390 100644
--- a/block.c
+++ b/block.c
@@ -5942,13 +5942,19 @@ bool bdrv_is_first_non_filter(BlockDriverState 
*candidate)
 BlockDriverState *check_to_replace_node(const char *node_name, Error **errp)
 {
 BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
+AioContext *aio_context;
+
 if (!to_replace_bs) {
 error_setg(errp, "Node name '%s' not found", node_name);
 return NULL;
 }
 
+aio_context = bdrv_get_aio_context(to_replace_bs);
+aio_context_acquire(aio_context);
+
 if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) {
-return NULL;
+to_replace_bs = NULL;
+goto out;
 }
 
 /* We don't want arbitrary node of the BDS chain to be replaced only the 
top
@@ -5958,9 +5964,12 @@ BlockDriverState *check_to_replace_node(const char 
*node_name, Error **errp)
  */
 if (!bdrv_is_first_non_filter(to_replace_bs)) {
 error_setg(errp, "Only top most non filter can be replaced");
-return NULL;
+to_replace_bs = NULL;
+goto out;
 }
 
+out:
+aio_context_release(aio_context);
 return to_replace_bs;
 }
 
diff --git a/block/mirror.c b/block/mirror.c
index 18b18e0..19b87d0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -314,9 +314,56 @@ static void mirror_drain(MirrorBlockJob *s)
 }
 }
 
+typedef struct {
+int ret;
+} MirrorExitData;
+
+static void mirror_exit(BlockJob *job, void *opaque)
+{
+MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+MirrorExitData *data = opaque;
+AioContext *replace_aio_context = NULL;
+
+if (s->to_replace) {
+replace_aio_context = bdrv_get_aio_context(s->to_replace);
+aio_context_acquire(replace_aio_context);
+}
+
+if (s->should_complete && data->ret == 0) {
+BlockDriverState *to_replace = s->common.bs;
+if (s->to_replace) {
+to_replace = s->to_replace;
+}
+if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
+bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
+}
+bdrv_swap(s->target, to_replace);
+if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
+/* drop the bs loop chain formed by the swap: break the loop then
+ * trigger the unref from the top one */
+BlockDriverState *p = s->base->backing_hd;
+bdrv_set_backing_hd(s->base, NULL);
+bdrv_unref(p);
+}
+}
+if (s->to_replace) {
+bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
+error_free(s->replace_blocker);
+bdrv_unref(s->to_replace);
+}
+if (replace_aio_context) {
+aio_context_release(replace_aio_context);
+}
+g_free(s->replaces);
+bdrv_unref(s->target);
+block_job_completed(&s->common, data->ret);
+g_free(data);
+}
+
 static void coroutine_fn mirror_run(void *opaque)
 {
 MirrorBlockJob *s = opaque;
+MirrorExitData *data;
 BlockDriverState *bs = s->common.bs;
 int64_t sector_num, end, sectors_per_chunk, length;
 uint64_t last_pause_ns;
@@ -467,7 +514,7 @@ static void coroutine_fn mirror_run(void *opaque)
  * mirror_populate runs.
  */
 trace_mirror_before_drain(s, cnt);
-bdrv_drain_all();
+bdrv_drain(bs);
 cnt = bdrv_get_dirty_count(bs, s->dirty_bitmap);
 }
 
@@ -510,31 +557,10 @@ immediate_exit:
 g_free(s->in_flight_bitmap);
 bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
 bdrv_iostatus_disable(s->target);
-if (s->should_complete && ret == 0) {
-BlockDriverState *to_replace = s->common.bs;
-if (s->to_replace) {
-to_replace = s->to_replace;
-}
-if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
-bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
-}
-bdrv_s

[Qemu-devel] [PATCH 07/11] block: let backup blockjob run in BDS AioContext

[Stefan Hajnoczi]

The backup block job must run in the BlockDriverState AioContext so that
it works with dataplane.

The basics of acquiring the AioContext are easy in blockdev.c.

The completion code in block/backup.c must call bdrv_unref() from the
main loop.  Use block_job_defer_to_main_loop() to achieve that.

Signed-off-by: Stefan Hajnoczi 
---
 block/backup.c | 21 +++--
 blockdev.c | 23 ---
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index d0b0225..9d015b5 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -227,9 +227,25 @@ static BlockErrorAction backup_error_action(BackupBlockJob 
*job,
 }
 }
 
+typedef struct {
+int ret;
+} BackupCompleteData;
+
+static void backup_complete(BlockJob *job, void *opaque)
+{
+BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+BackupCompleteData *data = opaque;
+
+bdrv_unref(s->target);
+
+block_job_completed(job, data->ret);
+g_free(data);
+}
+
 static void coroutine_fn backup_run(void *opaque)
 {
 BackupBlockJob *job = opaque;
+BackupCompleteData *data;
 BlockDriverState *bs = job->common.bs;
 BlockDriverState *target = job->target;
 BlockdevOnError on_target_error = job->on_target_error;
@@ -344,9 +360,10 @@ static void coroutine_fn backup_run(void *opaque)
 hbitmap_free(job->bitmap);
 
 bdrv_iostatus_disable(target);
-bdrv_unref(target);
 
-block_job_completed(&job->common, ret);
+data = g_malloc(sizeof(*data));
+data->ret = ret;
+block_job_defer_to_main_loop(&job->common, backup_complete, data);
 }
 
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
diff --git a/blockdev.c b/blockdev.c
index 1c79352..44f0cc7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2068,6 +2068,7 @@ void qmp_drive_backup(const char *device, const char 
*target,
 BlockDriverState *bs;
 BlockDriverState *target_bs;
 BlockDriverState *source = NULL;
+AioContext *aio_context;
 BlockDriver *drv = NULL;
 Error *local_err = NULL;
 int flags;
@@ -2093,9 +2094,12 @@ void qmp_drive_backup(const char *device, const char 
*target,
 return;
 }
 
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
 if (!bdrv_is_inserted(bs)) {
 error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-return;
+goto out;
 }
 
 if (!has_format) {
@@ -2105,12 +2109,12 @@ void qmp_drive_backup(const char *device, const char 
*target,
 drv = bdrv_find_format(format);
 if (!drv) {
 error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-return;
+goto out;
 }
 }
 
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
-return;
+goto out;
 }
 
 flags = bs->open_flags | BDRV_O_RDWR;
@@ -2130,7 +2134,7 @@ void qmp_drive_backup(const char *device, const char 
*target,
 size = bdrv_getlength(bs);
 if (size < 0) {
 error_setg_errno(errp, -size, "bdrv_getlength failed");
-return;
+goto out;
 }
 
 if (mode != NEW_IMAGE_MODE_EXISTING) {
@@ -2147,23 +2151,28 @@ void qmp_drive_backup(const char *device, const char 
*target,
 
 if (local_err) {
 error_propagate(errp, local_err);
-return;
+goto out;
 }
 
 target_bs = NULL;
 ret = bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &local_err);
 if (ret < 0) {
 error_propagate(errp, local_err);
-return;
+goto out;
 }
 
+bdrv_set_aio_context(target_bs, aio_context);
+
 backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
  block_job_cb, bs, &local_err);
 if (local_err != NULL) {
 bdrv_unref(target_bs);
 error_propagate(errp, local_err);
-return;
+goto out;
 }
+
+out:
+aio_context_release(aio_context);
 }
 
 BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
-- 
1.9.3

[Qemu-devel] [PATCH 06/11] block: add bdrv_drain()

[Stefan Hajnoczi]

Now that op blockers are in use, we can ensure that no other sources are
generating I/O on a BlockDriverState.  Therefore it is possible to drain
requests for a single BDS.

Signed-off-by: Stefan Hajnoczi 
---
 block.c   | 36 +---
 include/block/block.h |  1 +
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index c5a251c..48305c4 100644
--- a/block.c
+++ b/block.c
@@ -1918,6 +1918,34 @@ static bool bdrv_requests_pending(BlockDriverState *bs)
 return false;
 }
 
+static bool bdrv_drain_one(BlockDriverState *bs)
+{
+bool bs_busy;
+
+bdrv_flush_io_queue(bs);
+bdrv_start_throttled_reqs(bs);
+bs_busy = bdrv_requests_pending(bs);
+bs_busy |= aio_poll(bdrv_get_aio_context(bs), bs_busy);
+return bs_busy;
+}
+
+/*
+ * Wait for pending requests to complete on a single BlockDriverState subtree
+ *
+ * See the warning in bdrv_drain_all().  This function can only be called if
+ * you are sure nothing can generate I/O because you have op blockers
+ * installed.
+ *
+ * Note that unlike bdrv_drain_all(), the caller must hold the BlockDriverState
+ * AioContext.
+ */
+void bdrv_drain(BlockDriverState *bs)
+{
+while (bdrv_drain_one(bs)) {
+/* Keep iterating */
+}
+}
+
 /*
  * Wait for pending requests to complete across all BlockDriverStates
  *
@@ -1941,16 +1969,10 @@ void bdrv_drain_all(void)
 
 QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
 AioContext *aio_context = bdrv_get_aio_context(bs);
-bool bs_busy;
 
 aio_context_acquire(aio_context);
-bdrv_flush_io_queue(bs);
-bdrv_start_throttled_reqs(bs);
-bs_busy = bdrv_requests_pending(bs);
-bs_busy |= aio_poll(aio_context, bs_busy);
+busy |= bdrv_drain_one(bs);
 aio_context_release(aio_context);
-
-busy |= bs_busy;
 }
 }
 }
diff --git a/include/block/block.h b/include/block/block.h
index 3318f0d..61df804 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -372,6 +372,7 @@ int bdrv_flush(BlockDriverState *bs);
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
 int bdrv_flush_all(void);
 void bdrv_close_all(void);
+void bdrv_drain(BlockDriverState *bs);
 void bdrv_drain_all(void);
 
 int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
-- 
1.9.3

[Qemu-devel] [PATCH 03/11] blockdev: acquire AioContext in blockdev_mark_auto_del()

[Stefan Hajnoczi]

When an emulated storage controller is unrealized it will call
blockdev_mark_auto_del().  This will cancel any running block job (and
that eventually releases its reference to the BDS so it can be freed).

Since the block job may be executing in another AioContext we must
acquire/release to ensure thread safety.

Signed-off-by: Stefan Hajnoczi 
---
 blockdev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 270425d..5e38f34 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -89,14 +89,21 @@ static const int if_max_devs[IF_COUNT] = {
 void blockdev_mark_auto_del(BlockDriverState *bs)
 {
 DriveInfo *dinfo = drive_get_by_blockdev(bs);
+AioContext *aio_context;
 
 if (dinfo && !dinfo->enable_auto_del) {
 return;
 }
 
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
 if (bs->job) {
 block_job_cancel(bs->job);
 }
+
+aio_context_release(aio_context);
+
 if (dinfo) {
 dinfo->auto_del = 1;
 }
-- 
1.9.3

[Qemu-devel] [PATCH 04/11] blockdev: add note that block_job_cb() must be thread-safe

[Stefan Hajnoczi]

This function is correct but we should document the constraint that
everything must be thread-safe.

Emitting QMP events and scheduling BHs are both thread-safe so nothing
needs to be done here.

Signed-off-by: Stefan Hajnoczi 
---
 blockdev.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 5e38f34..1c79352 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1889,6 +1889,11 @@ out:
 
 static void block_job_cb(void *opaque, int ret)
 {
+/* Note that this function may be executed from another AioContext besides
+ * the QEMU main loop.  If you need to access anything that assumes the
+ * QEMU global mutex, use a BH or introduce a mutex.
+ */
+
 BlockDriverState *bs = opaque;
 const char *msg = NULL;
 
-- 
1.9.3

[Qemu-devel] [PATCH 08/11] block: let stream blockjob run in BDS AioContext

[Stefan Hajnoczi]

The stream block job must run in the BlockDriverState AioContext so that
it works with dataplane.

The basics of acquiring the AioContext are easy in blockdev.c.

The tricky part is the completion code which drops part of the backing
file chain.  This must be done in the main loop where bdrv_unref() and
bdrv_close() are safe to call.  Use block_job_defer_to_main_loop() to
achieve that.

Signed-off-by: Stefan Hajnoczi 
---
 block/stream.c | 50 --
 blockdev.c | 16 
 2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index cdea3e8..21bce12 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -79,9 +79,39 @@ static void close_unused_images(BlockDriverState *top, 
BlockDriverState *base,
 bdrv_refresh_limits(top, NULL);
 }
 
+typedef struct {
+int ret;
+bool reached_end;
+} StreamCompleteData;
+
+static void stream_complete(BlockJob *job, void *opaque)
+{
+StreamBlockJob *s = container_of(job, StreamBlockJob, common);
+StreamCompleteData *data = opaque;
+BlockDriverState *base = s->base;
+
+if (!block_job_is_cancelled(&s->common) && data->reached_end &&
+data->ret == 0) {
+const char *base_id = NULL, *base_fmt = NULL;
+if (base) {
+base_id = s->backing_file_str;
+if (base->drv) {
+base_fmt = base->drv->format_name;
+}
+}
+data->ret = bdrv_change_backing_file(job->bs, base_id, base_fmt);
+close_unused_images(job->bs, base, base_id);
+}
+
+g_free(s->backing_file_str);
+block_job_completed(&s->common, data->ret);
+g_free(data);
+}
+
 static void coroutine_fn stream_run(void *opaque)
 {
 StreamBlockJob *s = opaque;
+StreamCompleteData *data;
 BlockDriverState *bs = s->common.bs;
 BlockDriverState *base = s->base;
 int64_t sector_num, end;
@@ -183,21 +213,13 @@ wait:
 /* Do not remove the backing file if an error was there but ignored.  */
 ret = error;
 
-if (!block_job_is_cancelled(&s->common) && sector_num == end && ret == 0) {
-const char *base_id = NULL, *base_fmt = NULL;
-if (base) {
-base_id = s->backing_file_str;
-if (base->drv) {
-base_fmt = base->drv->format_name;
-}
-}
-ret = bdrv_change_backing_file(bs, base_id, base_fmt);
-close_unused_images(bs, base, base_id);
-}
-
 qemu_vfree(buf);
-g_free(s->backing_file_str);
-block_job_completed(&s->common, ret);
+
+/* Modify backing chain and close BDSes in main loop */
+data = g_malloc(sizeof(*data));
+data->ret = ret;
+data->reached_end = sector_num == end;
+block_job_defer_to_main_loop(&s->common, stream_complete, data);
 }
 
 static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp)
diff --git a/blockdev.c b/blockdev.c
index 44f0cc7..5473cd0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1923,6 +1923,7 @@ void qmp_block_stream(const char *device,
 {
 BlockDriverState *bs;
 BlockDriverState *base_bs = NULL;
+AioContext *aio_context;
 Error *local_err = NULL;
 const char *base_name = NULL;
 
@@ -1936,16 +1937,20 @@ void qmp_block_stream(const char *device,
 return;
 }
 
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
-return;
+goto out;
 }
 
 if (has_base) {
 base_bs = bdrv_find_backing_image(bs, base);
 if (base_bs == NULL) {
 error_set(errp, QERR_BASE_NOT_FOUND, base);
-return;
+goto out;
 }
+assert(bdrv_get_aio_context(base_bs) == aio_context);
 base_name = base;
 }
 
@@ -1954,7 +1959,7 @@ void qmp_block_stream(const char *device,
 if (base_bs == NULL && has_backing_file) {
 error_setg(errp, "backing file specified, but streaming the "
  "entire chain");
-return;
+goto out;
 }
 
 /* backing_file string overrides base bs filename */
@@ -1964,10 +1969,13 @@ void qmp_block_stream(const char *device,
  on_error, block_job_cb, bs, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
-return;
+goto out;
 }
 
 trace_qmp_block_stream(bs, bs->job);
+
+out:
+aio_context_release(aio_context);
 }
 
 void qmp_block_commit(const char *device,
-- 
1.9.3

[Qemu-devel] [PATCH 01/11] block: acquire AioContext in generic blockjob QMP commands

[Stefan Hajnoczi]

block-job-set-speed, block-job-cancel, block-job-pause,
block-job-resume, and block-job-complete must acquire the
BlockDriverState AioContext so that it is safe to access bs.

At the moment bs->job is always NULL when dataplane is active because op
blockers prevent blockjobs from starting.  Once the rest of the blockjob
API has been made aware of AioContext we can drop the op blocker.

Signed-off-by: Stefan Hajnoczi 
---
 blockdev.c | 52 +++-
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index ad43648..379a268 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2335,20 +2335,35 @@ void qmp_drive_mirror(const char *device, const char 
*target,
 }
 }
 
-static BlockJob *find_block_job(const char *device)
+/* Get the block job for a given device name and acquire its AioContext */
+static BlockJob *find_block_job(const char *device, AioContext **aio_context)
 {
 BlockDriverState *bs;
 
 bs = bdrv_find(device);
-if (!bs || !bs->job) {
-return NULL;
+if (!bs) {
+goto notfound;
+}
+
+*aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(*aio_context);
+
+if (!bs->job) {
+aio_context_release(*aio_context);
+goto notfound;
 }
+
 return bs->job;
+
+notfound:
+*aio_context = NULL;
+return NULL;
 }
 
 void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
 {
-BlockJob *job = find_block_job(device);
+AioContext *aio_context;
+BlockJob *job = find_block_job(device, &aio_context);
 
 if (!job) {
 error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
@@ -2356,34 +2371,40 @@ void qmp_block_job_set_speed(const char *device, 
int64_t speed, Error **errp)
 }
 
 block_job_set_speed(job, speed, errp);
+aio_context_release(aio_context);
 }
 
 void qmp_block_job_cancel(const char *device,
   bool has_force, bool force, Error **errp)
 {
-BlockJob *job = find_block_job(device);
-
-if (!has_force) {
-force = false;
-}
+AioContext *aio_context;
+BlockJob *job = find_block_job(device, &aio_context);
 
 if (!job) {
 error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
 return;
 }
+
+if (!has_force) {
+force = false;
+}
+
 if (job->paused && !force) {
 error_setg(errp, "The block job for device '%s' is currently paused",
device);
-return;
+goto out;
 }
 
 trace_qmp_block_job_cancel(job);
 block_job_cancel(job);
+out:
+aio_context_release(aio_context);
 }
 
 void qmp_block_job_pause(const char *device, Error **errp)
 {
-BlockJob *job = find_block_job(device);
+AioContext *aio_context;
+BlockJob *job = find_block_job(device, &aio_context);
 
 if (!job) {
 error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
@@ -2392,11 +2413,13 @@ void qmp_block_job_pause(const char *device, Error 
**errp)
 
 trace_qmp_block_job_pause(job);
 block_job_pause(job);
+aio_context_release(aio_context);
 }
 
 void qmp_block_job_resume(const char *device, Error **errp)
 {
-BlockJob *job = find_block_job(device);
+AioContext *aio_context;
+BlockJob *job = find_block_job(device, &aio_context);
 
 if (!job) {
 error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
@@ -2405,11 +2428,13 @@ void qmp_block_job_resume(const char *device, Error 
**errp)
 
 trace_qmp_block_job_resume(job);
 block_job_resume(job);
+aio_context_release(aio_context);
 }
 
 void qmp_block_job_complete(const char *device, Error **errp)
 {
-BlockJob *job = find_block_job(device);
+AioContext *aio_context;
+BlockJob *job = find_block_job(device, &aio_context);
 
 if (!job) {
 error_set(errp, QERR_BLOCK_JOB_NOT_ACTIVE, device);
@@ -2418,6 +2443,7 @@ void qmp_block_job_complete(const char *device, Error 
**errp)
 
 trace_qmp_block_job_complete(job);
 block_job_complete(job, errp);
+aio_context_release(aio_context);
 }
 
 void qmp_change_backing_file(const char *device,
-- 
1.9.3

[Qemu-devel] [PATCH 05/11] blockjob: add block_job_defer_to_main_loop()

[Stefan Hajnoczi]

Block jobs will run in the BlockDriverState's AioContext, which may not
always be the QEMU main loop.

There are some block layer APIs that are either not thread-safe or risk
lock ordering problems.  This includes bdrv_unref(), bdrv_close(), and
anything that calls bdrv_drain_all().

The block_job_defer_to_main_loop() API allows a block job to schedule a
function to run in the main loop with the BlockDriverState AioContext
held.

This function will be used to perform cleanup and backing chain
manipulations in block jobs.

Signed-off-by: Stefan Hajnoczi 
---
 blockjob.c   | 35 +++
 include/block/blockjob.h | 19 +++
 2 files changed, 54 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 0689fdd..24a64d8 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -313,3 +313,38 @@ BlockErrorAction block_job_error_action(BlockJob *job, 
BlockDriverState *bs,
 }
 return action;
 }
+
+typedef struct {
+BlockJob *job;
+QEMUBH *bh;
+AioContext *aio_context;
+BlockJobDeferToMainLoopFn *fn;
+void *opaque;
+} BlockJobDeferToMainLoopData;
+
+static void block_job_defer_to_main_loop_bh(void *opaque)
+{
+BlockJobDeferToMainLoopData *data = opaque;
+
+qemu_bh_delete(data->bh);
+
+aio_context_acquire(data->aio_context);
+data->fn(data->job, data->opaque);
+aio_context_release(data->aio_context);
+
+g_free(data);
+}
+
+void block_job_defer_to_main_loop(BlockJob *job,
+  BlockJobDeferToMainLoopFn *fn,
+  void *opaque)
+{
+BlockJobDeferToMainLoopData *data = g_malloc(sizeof(*data));
+data->job = job;
+data->bh = qemu_bh_new(block_job_defer_to_main_loop_bh, data);
+data->aio_context = bdrv_get_aio_context(job->bs);
+data->fn = fn;
+data->opaque = opaque;
+
+qemu_bh_schedule(data->bh);
+}
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 60aa835..5c13124 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -295,4 +295,23 @@ void block_job_iostatus_reset(BlockJob *job);
 BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
 BlockdevOnError on_err,
 int is_read, int error);
+
+typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque);
+
+/**
+ * block_job_defer_to_main_loop:
+ * @job: The job
+ * @fn: The function to run in the main loop
+ * @opaque: The opaque value that is passed to @fn
+ *
+ * Execute a given function in the main loop with the BlockDriverState
+ * AioContext acquired.  Block jobs must call bdrv_unref(), bdrv_close(), and
+ * anything that uses bdrv_drain_all() in the main loop.
+ *
+ * The @job AioContext is held while @fn executes.
+ */
+void block_job_defer_to_main_loop(BlockJob *job,
+  BlockJobDeferToMainLoopFn *fn,
+  void *opaque);
+
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 02/11] blockdev: acquire AioContext in do_qmp_query_block_jobs_one()

[Stefan Hajnoczi]

Make sure that query-block-jobs acquires the BlockDriverState
AioContext so that the blockjob isn't running in another thread while we
access its state.

Signed-off-by: Stefan Hajnoczi 
---
 blockdev.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 379a268..270425d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2585,14 +2585,19 @@ fail:
 static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
 {
 BlockJobInfoList **prev = opaque;
-BlockJob *job = bs->job;
+AioContext *aio_context;
+
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
 
-if (job) {
+if (bs->job) {
 BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
 elem->value = block_job_query(bs->job);
 (*prev)->next = elem;
 *prev = elem;
 }
+
+aio_context_release(aio_context);
 }
 
 BlockJobInfoList *qmp_query_block_jobs(Error **errp)
-- 
1.9.3

[Qemu-devel] [PATCH 00/11] block: allow blockjobs to coexist with dataplane

[Stefan Hajnoczi]

Almost all the infrastructure is in place to make blockjobs safe for use with
dataplane:

  * Op blockers all us to exclude commands that could conflict with a blockjob
or dataplane.

  * AioContext acquire/release allows threads to temporarily access a
BlockDriverState that is running in another thread.

This series introduces a few additional helpers:

  * block_job_defer_to_main_loop() which allows blockjobs to run their
completion code in the QEMU main loop.  This is necessary because some
operations are not safe outside the QEMU global mutex.

  * bdrv_drain() which can be used in limited cases to wait for in-flight
requests to complete (as opposed to the global bdrv_drain_all() function).

The approach taken in this series is to convert the blockdev.c monitor command
so it acquires the BlockDriverState's AioContext.  Normally only 1 AioContext
is involved at a time but the mirror job's to_replace argument can involve a
second AioContext.

Then the block job code itself is converted to defer main loop code using
block_job_defer_to_main_loop().

Example:

  $ qemu-system-x86_64 -enable-kvm -m 1024 \
-drive if=none,id=drive0,file=test.img \
-object iothread,iothread0 \
-device virtio-blk-pci,drive=drive0,iothread=iothread0
  (qemu) drive_mirror drive0 test2.img

Stefan Hajnoczi (11):
  block: acquire AioContext in generic blockjob QMP commands
  blockdev: acquire AioContext in do_qmp_query_block_jobs_one()
  blockdev: acquire AioContext in blockdev_mark_auto_del()
  blockdev: add note that block_job_cb() must be thread-safe
  blockjob: add block_job_defer_to_main_loop()
  block: add bdrv_drain()
  block: let backup blockjob run in BDS AioContext
  block: let stream blockjob run in BDS AioContext
  block: let mirror blockjob run in BDS AioContext
  block: let commit blockjob run in BDS AioContext
  block: declare blockjobs and dataplane friends!

 block.c |  49 +--
 block/backup.c  |  21 -
 block/commit.c  |  72 ++--
 block/mirror.c  |  85 +--
 block/stream.c  |  50 +++
 blockdev.c  | 179 +---
 blockjob.c  |  36 
 hw/block/dataplane/virtio-blk.c |   5 ++
 include/block/block.h   |   1 +
 include/block/blockjob.h|  19 +
 10 files changed, 394 insertions(+), 123 deletions(-)

-- 
1.9.3

Re: [Qemu-devel] [PATCH v4 12/23] block: Rename BlockDriverCompletionFunc to BlockCompletionFunc

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

I'll use it with block backends shortly, and the name is going to fit
badly there.  It's a block layer thing anyway, not just a block driver
thing.

Signed-off-by: Markus Armbruster 
---
  block.c | 30 +++---
  block/archipelago.c |  8 
  block/backup.c  |  2 +-
  block/blkdebug.c|  8 
  block/blkverify.c   |  8 
  block/commit.c  |  2 +-
  block/curl.c|  2 +-
  block/iscsi.c   |  2 +-
  block/linux-aio.c   |  2 +-
  block/mirror.c  |  6 +++---
  block/null.c|  8 
  block/qed-gencb.c   |  4 ++--
  block/qed-table.c   | 10 +-
  block/qed.c | 18 +-
  block/qed.h | 10 +-
  block/quorum.c  |  6 +++---
  block/raw-aio.h |  4 ++--
  block/raw-posix.c   | 16 
  block/raw-win32.c   |  8 
  block/raw_bsd.c |  2 +-
  block/rbd.c | 10 +-
  block/stream.c  |  2 +-
  block/win32-aio.c   |  2 +-
  blockjob.c  |  4 ++--
  dma-helpers.c   |  2 +-
  hw/ide/ahci.c   |  2 +-
  hw/ide/core.c   |  4 ++--
  hw/ide/internal.h   |  6 +++---
  hw/ide/macio.c  |  2 +-
  hw/ide/pci.c|  2 +-
  hw/ide/pci.h|  2 +-
  hw/scsi/scsi-generic.c  |  2 +-
  include/block/aio.h |  6 +++---
  include/block/block.h   | 14 +++---
  include/block/block_int.h   | 20 ++--
  include/block/blockjob.h|  4 ++--
  include/block/thread-pool.h |  2 +-
  include/monitor/monitor.h   |  4 ++--
  include/sysemu/dma.h|  8 
  monitor.c   |  6 +++---
  thread-pool.c   |  2 +-
  41 files changed, 131 insertions(+), 131 deletions(-)


I requested some trivial changes to patch 11 and rebasing this patch 
unto it will result in some trivial "functional" changes, if you heed my 
requests. Those should only be alignment changes, however (even the hunk 
in scsi-generic.c). In addition, docs/blkdebug.txt mentions 
BlockDriverCompletionFunc. Thus, like patch 11, this patch should 
replace that occurrence as well.


With these trivial changes (alignment changes due to rebasing on a patch 
11 changed according to my proposals and the replacement in 
docs/blkdebug.txt):


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v4 11/23] block: Rename BlockDriverAIOCB* to BlockAIOCB*

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

I'll use BlockDriverAIOCB with block backends shortly, and the name is
going to fit badly there.  It's a block layer thing anyway, not just a
block driver thing.

Signed-off-by: Markus Armbruster 
---
  block-migration.c   |   2 +-
  block.c | 124 ++--
  block/archipelago.c |  10 ++--
  block/blkdebug.c|  10 ++--
  block/blkverify.c   |   8 +--
  block/curl.c|   4 +-
  block/iscsi.c   |   6 +--
  block/linux-aio.c   |   6 +--
  block/null.c|  10 ++--
  block/qed.c |   8 +--
  block/qed.h |   2 +-
  block/quorum.c  |  36 ++---
  block/raw-aio.h |   4 +-
  block/raw-posix.c   |  16 +++---
  block/raw-win32.c   |   8 +--
  block/raw_bsd.c |   8 +--
  block/rbd.c |  12 ++---
  block/sheepdog.c|   4 +-
  block/win32-aio.c   |   4 +-
  dma-helpers.c   |  12 ++---
  hw/block/nvme.h |   2 +-
  hw/ide/ahci.h   |   2 +-
  hw/ide/core.c   |   8 +--
  hw/ide/internal.h   |   6 +--
  hw/ppc/mac.h|   2 +-
  hw/scsi/scsi-generic.c  |   2 +-
  include/block/aio.h |   8 +--
  include/block/block.h   |  20 +++
  include/block/block_int.h   |  10 ++--
  include/block/thread-pool.h |   2 +-
  include/hw/scsi/scsi.h  |   2 +-
  include/sysemu/dma.h|  26 +-
  tests/test-thread-pool.c|   2 +-
  thread-pool.c   |   8 +--
  34 files changed, 197 insertions(+), 197 deletions(-)



[snip]


diff --git a/block/archipelago.c b/block/archipelago.c
index 73d91a4..edbfbb5 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -86,7 +86,7 @@ typedef enum {
  } ARCHIPCmd;
  
  typedef struct ArchipelagoAIOCB {

-BlockDriverAIOCB common;
+BlockAIOCB common;
  QEMUBH *bh;
  struct BDRVArchipelagoState *s;
  QEMUIOVector *qiov;
@@ -856,7 +856,7 @@ err_exit:
  return ret;
  }
  
-static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs,

+static BlockAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs,
   int64_t sector_num,
   QEMUIOVector *qiov,
   int nb_sectors,


This breaks the alignment.

[snip]


diff --git a/block/blkverify.c b/block/blkverify.c
index 7d64a23..a29ed05 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c


[snip]


-static BlockDriverAIOCB *blkverify_aio_flush(BlockDriverState *bs,
+static BlockAIOCB *blkverify_aio_flush(BlockDriverState *bs,
   BlockDriverCompletionFunc *cb,
   void *opaque)
  {


Breaks the alignment.

[snip]


diff --git a/block/null.c b/block/null.c
index 8284419..b353a73 100644
--- a/block/null.c
+++ b/block/null.c


[snip]


@@ -94,7 +94,7 @@ static void null_bh_cb(void *opaque)
  qemu_aio_unref(acb);
  }
  
-static inline BlockDriverAIOCB *null_aio_common(BlockDriverState *bs,

+static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
  BlockDriverCompletionFunc *cb,
  void *opaque)
  {


Alignment again (applies to all hunks in block/null.c touching function 
headers).


[snip]


diff --git a/block/qed.c b/block/qed.c
index 7f03c26..0382228 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1365,7 +1365,7 @@ static void qed_aio_next_io(void *opaque, int ret)
io_fn, acb);
  }
  
-static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs,

+static BlockAIOCB *qed_aio_setup(BlockDriverState *bs,
 int64_t sector_num,
 QEMUIOVector *qiov, int nb_sectors,
 BlockDriverCompletionFunc *cb,


Alignment, for all function header hunks in block/qed.c.

[snip]


diff --git a/block/rbd.c b/block/rbd.c
index 96947e3..f44a093 100644
--- a/block/rbd.c
+++ b/block/rbd.c


[snip]


@@ -589,7 +589,7 @@ static int rbd_aio_flush_wrapper(rbd_image_t image,
  #endif
  }
  
-static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,

+static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
 int64_t sector_num,
 QEMUIOVector *qiov,
 int nb_sectors,


Alignment, for all function header hunks in block/rbd.c.

[snip]


diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 20587b4..8baee16 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -157,7 +157,7 @@ static void scsi_cancel_io(SCSIRequest *req)
  
  static int 

Re: [Qemu-devel] [PATCH] virtio-balloon: Tweak recent fix for integer overflow

[Eric Blake]

On 10/01/2014 10:43 AM, Markus Armbruster wrote:
> Commit 1f9296b avoids "other kinds of overflow" by limiting the
> polling interval to UINT_MAX.  The computations to protect are done in
> 64 bits.  This is indeed safe when unsigned is 32 bits, as it commonly
> is.  It isn't when unsigned is 64 bits.  Purely theoretical; I'm not
> aware of such a system.  Limit it to UINT32_MAX instead.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  hw/virtio/virtio-balloon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake 

Harmless sanity addition (I seriously doubt at this point that anyone
would ever introduce a platform where 'int' is larger than 32 bits)

> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index b5cf7ca..7bfbb75 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -170,7 +170,7 @@ static void balloon_stats_set_poll_interval(Object *obj, 
> struct Visitor *v,
>  return;
>  }
>  
> -if (value > UINT_MAX) {
> +if (value > UINT32_MAX) {
>  error_setg(errp, "timer value is too big");
>  return;
>  }
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] virtio-balloon: Tweak recent fix for integer overflow

[Markus Armbruster]

Commit 1f9296b avoids "other kinds of overflow" by limiting the
polling interval to UINT_MAX.  The computations to protect are done in
64 bits.  This is indeed safe when unsigned is 32 bits, as it commonly
is.  It isn't when unsigned is 64 bits.  Purely theoretical; I'm not
aware of such a system.  Limit it to UINT32_MAX instead.

Signed-off-by: Markus Armbruster 
---
 hw/virtio/virtio-balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index b5cf7ca..7bfbb75 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -170,7 +170,7 @@ static void balloon_stats_set_poll_interval(Object *obj, 
struct Visitor *v,
 return;
 }
 
-if (value > UINT_MAX) {
+if (value > UINT32_MAX) {
 error_setg(errp, "timer value is too big");
 return;
 }
-- 
1.9.3

Re: [Qemu-devel] Request for help

[Christopher Covington]

Hi Martin,

On 10/01/2014 09:50 AM, Martin Townsend wrote:
> Hi,
> 
> I'm looking into creating a virtualised test bed for an 802.15.4 network. 
> Currently I have QEMU running and emulating our HW which bridges the
> Ethernet from the guest to the host. What I would like to do is something
> similar with the 802.15.4 network interface. The host doesn't have a
> physical 802.15.4 interface but if I could get the data out of the guest
> and into QEMU I could communicate with a central network simulator to deal
> with the packets.
>
> My first thought was to write a virtual device for QEMU that would 
> interface with our 802.15.4 driver in the guest but then I realise that the
> real device hangs of an AMBA bus

What exactly is your objection to this? Aren't there plenty of "AMBA bus"
peripherals that you can reference attached to the Versatile Express for 
example?

> so my next idea was to write a PCI driver for 802.15.4 on the guest making
> it possible to write a PCI virtual QEMU device.
> 
> I just wanted to confirm that this is possible and whether there is a
> better solution?

Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Re: [Qemu-devel] pending target-mips patches

[Andreas Färber]

Hi Leon,

Am 01.10.2014 um 17:35 schrieb Leon Alrae:
> I noticed that it's quite difficult to get target-mips changes
> reviewed/accepted. There is already a queue of relatively big features
> and bug fixes which are stuck for months. Does anyone have an idea how
> to improve this situation? Wouldn't it help to have a target-mips
> co-maintainer assisting Aurelien?

In February I talked to one of your directors and suggested whether
someone from Imagination could step up as co-maintainer to tackle this.
I admit, we never followed up on that conversation so far...

So, from my view what someone should do by now is this:

* Set up one or more public Git branches for officially queuing
target-mips/ and hw/mips/ patches. That helps track pending patches and
facilitates testing. I assume you have some shared internal tree anyway,
just split between what that maintainer considers good and what is still
work-in-progress.

* From time to time, send a PULL request that either Aurélien or Peter
can merge. Aurélien used to commit patches himself traditionally,
whereas I am suggesting to adopt the workflow used for ARM and Power.

The underlying assumption is that such MIPS patches would not touch
generic code without explicit ACKs, thereby not breaking x86 code.

Also, as usual, the person needs to have been around qemu-devel a little
and beware of what common style/functional issues to look out for and of
what legacy machines/CPUs exist that might break when implementing new
stuff - having test images to verify would be ideal.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] pending target-mips patches

[Peter Maydell]

On 1 October 2014 16:35, Leon Alrae  wrote:
> I noticed that it's quite difficult to get target-mips changes
> reviewed/accepted. There is already a queue of relatively big features
> and bug fixes which are stuck for months. Does anyone have an idea how
> to improve this situation? Wouldn't it help to have a target-mips
> co-maintainer assisting Aurelien?

I agree that an active co-maintainer for target-mips would
be a good idea. Is anybody volunteering? (Are you? :-))

If somebody wants to take on this job, a good first start
would be to assemble a git tree of patches which have got
code review (ie which have accumulated at least one
reviewed-by tag and aren't the subject of on-list
disagreement about whether they're correct), test it, and
submit it as a pull request (in the right format, and with
your signed-off-by lines on the patches and any reviewed-by
or acked-by lines from the list)... If somebody does that
I will take a look at the result and if I'm happy with it
I'll apply them.

thanks
-- PMM

Re: [Qemu-devel] [PATCHv2 1/2] util: introduce bitmap_try_new

[Dr. David Alan Gilbert]

* Peter Lieven (p...@kamp.de) wrote:
> regular bitmap_new simply aborts if the memory allocation fails.
> bitmap_try_new returns NULL on failure and allows for proper
> error handling.
> 
> Signed-off-by: Peter Lieven 
> ---
>  include/qemu/bitmap.h |   13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index 1babd5d..edf4f17 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -88,10 +88,19 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned 
> long *bitmap1,
>  int slow_bitmap_intersects(const unsigned long *bitmap1,
> const unsigned long *bitmap2, long bits);
>  
> -static inline unsigned long *bitmap_new(long nbits)
> +static inline unsigned long *bitmap_try_new(long nbits)
>  {
>  long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
> -return g_malloc0(len);
> +return g_try_malloc0(len);
> +}
> +
> +static inline unsigned long *bitmap_new(long nbits)
> +{
> +unsigned long *ptr = bitmap_try_new(nbits);
> +if (ptr == NULL) {
> +abort();
> +}
> +return ptr;
>  }

This seems like a good idea; it's probably a good idea to deprecate
use of bitmap_new and get rid of the other uses in the longterm
(there aren't that many).

size_t would be a nice type for the nbits; could you make your bitmap_try_new
use that and then fix up those users as we convert them to use the try?

Dave

>  
>  static inline void bitmap_zero(unsigned long *dst, long nbits)
> -- 
> 1.7.9.5
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 4/7] tests: Add unit test for X86CPU code

[Eduardo Habkost]

On Wed, Oct 01, 2014 at 12:20:18AM +0200, Paolo Bonzini wrote:
> Il 30/09/2014 20:26, Eduardo Habkost ha scritto:
> > Reasoning for each object file included in the test binary:
> >  * qom/cpu.o - for TYPE_CPU. Dependencies:
> >* qom/qom-qobject.o
> >  * qom/qdev.o - for TYPE_DEVICE. Dependencies:
> >* qom/container.o
> >* vmstate.o. Dependencies:
> >  * qemu-file.o
> >* hw/core/hotplug.o
> >* hw/core/irq.o
> >* hw/core/fw-path-provider.o
> >* hw/core/qdev-properties.o
> >  * qom/object.o - for TYPE_OBJECT
> >  * x86_64-softmmu/target-i386/machine.o - for vmstate_x86_cpu
> >  * qemu-log.o - for the logging API, used by target-i386/cpu.c
> >  * libqemuutil.a - for QAPI visitors, error API, and other symbols
> >  * libqemustub.a - existing stubs, including: savevm, monitor symbols
> > 
> > The remaining symbols used by target-i386/cpu.c were added as stubs to
> > either tests/vl-stub.c and tests/x86-stub.c.
> 
> Nice.  Luckily qemu-log.o doesn't bring in everything.
> 
> I think vl-stub.c has to be re-evaluated after your QOM accelerator
> patch goes in.
> 
> tests/x86-stub.c perhaps can be moved to target-i386/test-stubs.c?

I was trying to keep all test code inside tests/. But perhaps all the
target-specific test code (including test-x86-cpu.c) could be moved to
target directories, and we could build/run the target-specific test
cases from Makefile.target. That should simplify some of the logic I
have added, and fix the build dependency problem I mentioned in the
patch description.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v4 09/23] block: Merge BlockBackend and BlockDriverState name spaces

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

BlockBackend's name space is separate only to keep the initial patches
simple.  Time to merge the two.

Retain bdrv_find() and bdrv_get_device_name() for now, to keep this
series manageable.

Signed-off-by: Markus Armbruster 
---
  block.c   | 48 
  block/block-backend.c | 17 +++--
  include/block/block.h |  2 +-
  3 files changed, 24 insertions(+), 43 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v4 08/23] block: Eliminate BlockDriverState member device_name[]

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

device_name[] can become non-empty only in bdrv_new_root() and
bdrv_move_feature_fields().  The latter is used only to undo damage
done by bdrv_swap().  The former is called only by blk_new_with_bs().
Therefore, when a BlockDriverState's device_name[] is non-empty, then
it's been created with a BlockBackend, and vice versa.  Furthermore,
blk_new_with_bs() keeps the two names equal.

Therefore, device_name[] is redundant.  Eliminate it.

Signed-off-by: Markus Armbruster 
---
  block-migration.c | 12 +++-
  block.c   | 49 ++-
  block/mirror.c|  3 ++-
  block/qapi.c  |  6 +++---
  block/qcow.c  |  4 ++--
  block/qcow2.c |  4 ++--
  block/qed.c   |  2 +-
  block/quorum.c|  4 ++--
  block/vdi.c   |  2 +-
  block/vhdx.c  |  2 +-
  block/vmdk.c  |  4 ++--
  block/vpc.c   |  2 +-
  block/vvfat.c |  2 +-
  blockjob.c|  3 ++-
  include/block/block.h |  2 +-
  include/block/block_int.h |  2 --
  16 files changed, 55 insertions(+), 48 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v2 1/7] tests: Move fake yield_until_fd_readable() to coroutine-stub.c

[Eduardo Habkost]

On Wed, Oct 01, 2014 at 12:17:50AM +0200, Paolo Bonzini wrote:
> Il 30/09/2014 20:26, Eduardo Habkost ha scritto:
> > Other test code will use the function.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  tests/Makefile |  1 +
> >  tests/coroutine-stub.c | 13 +
> >  tests/test-vmstate.c   | 11 ---
> >  3 files changed, 14 insertions(+), 11 deletions(-)
> >  create mode 100644 tests/coroutine-stub.c
> 
> Should we split qemu-file.c instead?

Splitting qemu-file.c won't avoid the need for a fake
yield_until_fd_readable() for test-vmstate.c, but will probably help
reduce cpu.c dependencies. I will give it a try.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 5/5] target-tricore: Add instructions of BO opcode format

[Richard Henderson]

On 10/01/2014 02:35 AM, Bastian Koppelmann wrote:
> +case OPC2_32_BO_ST_A_PREINC:
> +tcg_gen_addi_tl(cpu_gpr_a[r2], cpu_gpr_a[r2], off10);
> +tcg_gen_qemu_st_tl(cpu_gpr_a[r1], cpu_gpr_a[r2], ctx->mem_idx,
> +   MO_LESL);
> +break;

The writeback to A[r2] should not happen until after the memory operation, so
that if a memory exception occurs, A[r2] is not adjusted.


r~

Re: [Qemu-devel] [Xen-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport

[Anthony Liguori]

On Wed, Oct 1, 2014 at 7:44 AM, Ian Campbell  wrote:
> On Wed, 2014-10-01 at 10:20 +0100, Stefano Stabellini wrote:
>> I wonder if we could send both ioreqs at once from Xen and back from
>> QEMU. Or maybe append the registers to IOREQ_TYPE_VMWARE_PORT, changing
>> the size of ioreq_t only for this ioreq type.
>
> Random idea: Why new add a IOREQ_TYPE_FULL_STATE which would be issued
> for these ports and let qemu decode the fact that it is vmware
> internally? That might be a more generically useful interface in the
> future?
>
> WRT to fitting all the register state in the current sized request, you
> could declare that this new thing takes multiple slots.
>
> Also, I may be wrong, but I thought most IOREQs were synchronous so only
> one slot was ever used? The buffered ioreq stuff has a separate ring (or
> uses a different part of the page, or something). I might be talking
> nonsense here though ;-)

There really isn't a concept of "CPU associated with an IOREQ" outside
of two very special cases, LAPIC emulation and vmport.  LAPIC
emulation really belongs closer to the CPU and given V-APIC, it's
gotten moved into hardware anyway.  vmport is just a hack VMware made.

I think it's better to think of it as a VMware specific hypercall and
terminate the IOREQ within the hypervisor.  Passing a decoded version
of the request to QEMU is fine but passing the full CPU state as part
of an IOREQ_TYPE_FULL_STATE is not very useful.  It's just an
IOREQ_TYPE_VMPORT with more information than is needed.

Regards,

Anthony Liguori

> Ian.
>
>
>
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [Qemu-devel] [PATCH v4 04/23] block: Connect BlockBackend and DriveInfo

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

Make the BlockBackend own the DriveInfo.  Change blockdev_init() to
return the BlockBackend instead of the DriveInfo.

Signed-off-by: Markus Armbruster 
---
  block.c   |  2 --
  block/block-backend.c | 38 ++
  blockdev.c| 69 ---
  include/sysemu/blockdev.h |  4 +++
  4 files changed, 77 insertions(+), 36 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v2 4/5] target-tricore: Add instructions of BIT opcode format

[Richard Henderson]

On 10/01/2014 02:35 AM, Bastian Koppelmann wrote:
> +case OPC2_32_BIT_AND_NOR_T:
> +#if defined TCG_TARGET_HAS_nor_i32
> +gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
> +pos1, pos2, &tcg_gen_nor_tl, &tcg_gen_and_tl);
> +#else

These are *always* defined.  You want a normal C if test.

That said, I would actually write it the other way:

   if (TCG_TARGET_HAS_andc_i32) {
   gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
   pos1, pos2, &tcg_gen_or_tl, &tcg_gen_andc_tl);
   } else {
   gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
   pos1, pos2, &tcg_gen_nor_tl, &tcg_gen_and_tl);
   }

so that the "normal" case is the one that matches the opcode name.


r~

Re: [Qemu-devel] [PATCH v2 2/5] target-tricore: Add instructions of ABS, ABSB opcode format

[Richard Henderson]

On 10/01/2014 02:35 AM, Bastian Koppelmann wrote:
> Add instructions of ABS, ABSB opcode format.
> Add microcode generator functions for ld/st of two 32bit reg as one 64bit 
> value.
> Add microcode generator functions for ldmst and swap.
> Add helper ldlcx, lducx, stlcx and stucx.
> 
> Signed-off-by: Bastian Koppelmann 
> ---
> v1 -> v2:
> - Fix whitespaces
> - gen_ld_2regs_64: replace three tcg-ops to write back 64bit result with 
> tcg_gen_extr
> - decode32Bit: move declaration of b and bpos to the top of the function.

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 2/2] block: Make op blockers recursive

[Benoît Canet]

> > 
> > The main purpose of this is mirror.c and commit.c would form BDS loops on 
> > completion.
> > These callers could break the look manually but the code would fail
> > if a loop is not breaked and the blocker function are called on it.
> > So the blocker code have to handle recursion loops.
> 
> I think commit/mirror/etc should break any loops prior to calling
> recursive functions on those loops (just like it should do before
> calling bdrv_unref(), etc..).  Otherwise, I think the recursive
> functions make assumptions that may be true in certain contexts, but
> not necessarily all.
> 
> (Hmm, I believe that Fam had a series that did some nice cleanup on
> bdrv_drop_intermediate() and related areas that did some loop
> breaking, IIRC).

Ok I could use that as a basis.

> > 
> > I don't think this particular test is a failure point.
> >
> 
> With the way it is written, the individual BDS is blocked with the
> same reason pointer, but not necessarily the whole chain - unless you
> make the assumption that blockers will only be used via the recursive
> interface, and never individually on a node.

there is no more a no recursive version with this patch so this assumption
will be respected.

> 
> The caller doesn't have an interface to check that the chain is not
> blocked - bdrv_op_is_blocked_by() doesn't operate recursively.  
> 
> If we tried to block a chain that has some portion already blocked for
> the same reason, shouldn't that be an error?

Why should we allow this ?
My understanding is that blocking something should be associated to a
single operation whatever they are.
So one operation to block implying one different reason is not so strange.

> > > > +
> > > > +/* block first for recursion loop protection to work */
> > > > +bdrv_do_op_block(bs, op, reason);
> > > > +
> > > > +bdrv_op_block(bs->file, base, op, reason);
> > > > +
> > > > +if (bs->drv && bs->drv->supports_backing) {
> > > > +bdrv_op_block(bs->backing_hd, base, op, reason);
> > > > +}
> > > > +
> > > > +if (bs->drv && bs->drv->bdrv_op_recursive_block) {
> > > > +bs->drv->bdrv_op_recursive_block(bs, base, op, reason);
> > > 
> > > Do we need to allow .bdrv_op_recursive_block() to fail and return
> > > error (and handle it, of course)?
> > 
> > I don't know yet: but lets let this question float a little more in the 
> > mail thread.
> > 
> > > 
> > > 
> > > 


> > 
> > To reach this state the caller code would have to do the following twisted 
> > sequence.
> > 
> > block(image3, with_reason1)
> > unblock(image2, with_reason1)
> > block(image1, with_reason1)
> >
> > There is no such sequence in the code thanks to the base argument and we 
> > could
> > enforce that no such sequence ever get written.
> >
> 
> If we ignore blockdev-add and scenarios where an image node may have
> multiple overlays, we might be able to assume that such a sequence
> could not exist.
> 
> But in that case, should this negative check result in an error?
> 
> It would seem at this point we would have encountered one of these
> scenarios:
> 
> 1.) An invalid block/unblock state in the chain, if we assume that no
> such sequence should exist
> 
> 2.) We tried to unblock more than we originally blocked

> 
> > > 
> > > I would assume that bdrv_op_unblock(image2, NULL, reason) would still
> > > unblock image1, even if image2 was unblocked.
> > 
> > Should we also assume that bdrv_op_unblock(image4, NULL, reason) with 
> > image4 being
> > image3 parent unblock everything underneath ?
> > 
> 
> I think we either do that, or return an error.  But to have
> bdrv_op_unblock() (or bdrv_op_block()) silently stop at some point in
> the chain prior to reaching 'base' doesn't seem correct to me.
> 

Maybe you are right.
I don't mind rewriting the patchset with error handling and without the 
recursion
loop avoidance code given I find Fam's loop breaking patches on the list.

I remember trying to write loop breaking by myself just before 2.1 and it was
annoying.

Best regards

Benoît

[Qemu-devel] pending target-mips patches

[Leon Alrae]

Hi All,

I noticed that it's quite difficult to get target-mips changes
reviewed/accepted. There is already a queue of relatively big features
and bug fixes which are stuck for months. Does anyone have an idea how
to improve this situation? Wouldn't it help to have a target-mips
co-maintainer assisting Aurelien?

Leon

Re: [Qemu-devel] [PATCH v4 03/23] block: Connect BlockBackend to BlockDriverState

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

Convenience function blk_new_with_bs() creates a BlockBackend with its
BlockDriverState.  Callers have to unref both.  The commit after next
will relieve them of the need to unref the BlockDriverState.

Complication: due to the silly way drive_del works, we need a way to
hide a BlockBackend, just like bdrv_make_anon().  To emphasize its
"special" status, give the function a suitably off-putting name:
blk_hide_on_behalf_of_do_drive_del().  Unfortunately, hiding turns the
BlockBackend's name into the empty string.  Can't avoid that without
breaking the blk->bs->device_name equals blk->name invariant.

The patch adds a memory leak: drive_del while a device model is
connected leaks the BlockBackend.  Avoiding the leak here is rather
hairy, but it'll become straightforward in a few commits, so I mark it
FIXME in the code now, and plug it when it's easy.

Signed-off-by: Markus Armbruster 
---
  block.c|  12 ++--
  block/block-backend.c  |  71 ++-
  blockdev.c |  18 +++---
  hw/block/xen_disk.c|   8 +--
  include/block/block_int.h  |   2 +
  include/sysemu/block-backend.h |   5 ++
  qemu-img.c | 125 +++--
  qemu-io.c  |   4 +-
  qemu-nbd.c |   4 +-
  9 files changed, 156 insertions(+), 93 deletions(-)


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v4 02/23] block: New BlockBackend

[Max Reitz]

On 30.09.2014 21:25, Markus Armbruster wrote:

A block device consists of a frontend device model and a backend.

A block backend has a tree of block drivers doing the actual work.
The tree is managed by the block layer.

We currently use a single abstraction BlockDriverState both for tree
nodes and the backend as a whole.  Drawbacks:

* Its API includes both stuff that makes sense only at the block
   backend level (root of the tree) and stuff that's only for use
   within the block layer.  This makes the API bigger and more complex
   than necessary.  Moreover, it's not obvious which interfaces are
   meant for device models, and which really aren't.

* Since device models keep a reference to their backend, the backend
   object can't just be destroyed.  But for media change, we need to
   replace the tree.  Our solution is to make the BlockDriverState
   generic, with actual driver state in a separate object, pointed to
   by member opaque.  That lets us replace the tree by deinitializing
   and reinitializing its root.  This special need of the root makes
   the data structure awkward everywhere in the tree.

The general plan is to separate the APIs into "block backend", for use
by device models, monitor and whatever other code dealing with block
backends, and "block driver", for use by the block layer and whatever
other code (if any) dealing with trees and tree nodes.

Code dealing with block backends, device models in particular, should
become completely oblivious of BlockDriverState.  This should let us
clean up both APIs, and the tree data structures.

This commit is a first step.  It creates a minimal "block backend"
API: type BlockBackend and functions to create, destroy and find them.

BlockBackend objects are created and destroyed exactly when root
BlockDriverState objects are created and destroyed.  "Root" in the
sense of "in bdrv_states".  They're not yet used for anything; that'll
come shortly.

BlockBackend is reference-counted.  Its reference count never exceeds
one so far, but that's going to change.

Signed-off-by: Markus Armbruster 
---
  block/Makefile.objs|   2 +-
  block/block-backend.c  | 120 +
  blockdev.c |  13 -
  hw/block/xen_disk.c|  11 
  include/qemu/typedefs.h|   1 +
  include/sysemu/block-backend.h |  26 +
  qemu-img.c |  70 +---
  qemu-io.c  |   8 +++
  qemu-nbd.c |   5 +-
  9 files changed, 245 insertions(+), 11 deletions(-)
  create mode 100644 block/block-backend.c
  create mode 100644 include/sysemu/block-backend.h


Reviewed-by: Max Reitz 

Re: [Qemu-devel] [PATCH v2 2/2] block: Make op blockers recursive

[Jeff Cody]

On Wed, Oct 01, 2014 at 09:29:44AM +, Benoît Canet wrote:
> 
> Thanks a lot for reviewing this patch.
> 
> Since the code is not trivial I will give my arguments for writing it
> this way.
>

Thanks, that is very helpful.

> 
> > > +/* block recursively a BDS
> > > + *
> > > + * If base != NULL the caller must make sure that there is no multiple 
> > > child
> > > + * BDS in the subtree pointed to by bs
> > 
> > In the case when base != NULL, should that really matter?  In a
> > driver's .bdrv_op_recursive_block/unblock, if that driver has private
> > children (multiple or not), shouldn't the private children be treated
> > as one black box, and blocked / unblocked just like the parent
> > BDS?
> 
> This is a stale comment. My bad.
> 
> > 
> > For instance, what if we had a tree such as this:
> > 
> >[quorum1] < [active]
> >|
> >| (private)
> >|
> >v
> > [node2]<-- [node1] <--- [imag> 
> > if 'quorum1' was to be op blocked, and 'image1' and its children all
> > comprise 'quorum1', wouldn't we always want to lock 'image1' all the
> > way down to 'node2'?
> 
> That's what the patch does.
> 

OK, great - my comment above was written in response to the stale comment :)

> > 
> > Or do we expect that someone will intentionally pass a 'base' pointer
> > along that matches somewhere in the private child tree?
> 
> This is not expected by the caller but I wrote the patch so it will also work.
> 
> > 
> > > + *
> > > + * @bs: the BDS to start to recurse from
> > > + * @base:   the BDS where the recursion should end
> > > + *  could be NULL if the recursion should go down everywhere
> > > + * @op: the type of op blocker to block
> > > + * @reason: the error message associated with the blocking
> > > + */
> > > +void bdrv_op_block(BlockDriverState *bs, BlockDriverState *base,
> > > +   BlockOpType op, Error *reason)
> > > +{
> > > +if (!bs) {
> > > +return;
> > > +}
> > > +
> > > +/* did we recurse down to base ? */
> > > +if (bs == base) {
> > > +return;
> > > +}
> > > +
> > > +/* prevent recursion loop */
> > > +if (bdrv_op_is_blocked_by(bs, op, reason)) {
> > > +return;
> > > +}
> > 
> > Should we handle this somehow (isn't this effectively an error, that
> > will prematurely end the blocking of this particular line)? 
> 
> The main purpose of this is mirror.c and commit.c would form BDS loops on 
> completion.
> These callers could break the look manually but the code would fail
> if a loop is not breaked and the blocker function are called on it.
> So the blocker code have to handle recursion loops.

I think commit/mirror/etc should break any loops prior to calling
recursive functions on those loops (just like it should do before
calling bdrv_unref(), etc..).  Otherwise, I think the recursive
functions make assumptions that may be true in certain contexts, but
not necessarily all.

(Hmm, I believe that Fam had a series that did some nice cleanup on
bdrv_drop_intermediate() and related areas that did some loop
breaking, IIRC).

> 
> The bdrv_op_is_blocked_by match a reason being the criteria to test the 
> blocker.
> 
> So this test will trigger only if a BDS is already blocked by the same reason
> pointer.
> 
> This make the bdrv_op_block function idempotent.
> 
> note that it is the only sane way I found to make the blockers function handle
> loops.
> 
> > 
> > If we hit this, we are going to end up in a scenario where we haven't
> > blocked the chain as requested, and we don't know the state of the
> > blocking below this failure point.  Seems like the caller should know,
> > and we should have a way of cleaning up.
> 
> If we hit this the chain was already blocked with the same reason pointer.
> 
> > 
> > Actually, now I am wondering if we perhaps shouldn't be building a
> > list to block, and then blocking everything atomically once we know
> > there are no failure points.
> >
> 
> I don't think this particular test is a failure point.
>

With the way it is written, the individual BDS is blocked with the
same reason pointer, but not necessarily the whole chain - unless you
make the assumption that blockers will only be used via the recursive
interface, and never individually on a node.

The caller doesn't have an interface to check that the chain is not
blocked - bdrv_op_is_blocked_by() doesn't operate recursively.  

If we tried to block a chain that has some portion already blocked for
the same reason, shouldn't that be an error?


> > > +
> > > +/* block first for recursion loop protection to work */
> > > +bdrv_do_op_block(bs, op, reason);
> > > +
> > > +bdrv_op_block(bs->file, base, op, reason);
> > > +
> > > +if (bs->drv && bs->drv->supports_backing) {
> > > +bdrv_op_block(bs->backing_hd, base, op, reason);
> > > +}
> > > +
> > > +if (bs->drv 

Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport

[Paul Durrant]

> -Original Message-
> From: qemu-devel-bounces+paul.durrant=citrix@nongnu.org
> [mailto:qemu-devel-bounces+paul.durrant=citrix@nongnu.org] On
> Behalf Of Stefano Stabellini
> Sent: 01 October 2014 10:20
> To: Slutz, Donald Christopher
> Cc: xen-de...@lists.xensource.com; Stefano Stabellini; Markus Armbruster;
> Marcel Apfelbaum; Alexander Graf; qemu-devel@nongnu.org; Michael S.
> Tsirkin; Anthony Liguori; Andreas Färber
> Subject: Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen
> access to vmport
> 
> On Wed, 1 Oct 2014, Slutz, Donald Christopher wrote:
> > On 09/30/14 06:35, Stefano Stabellini wrote:
> > > On Mon, 29 Sep 2014, Don Slutz wrote:
> > >> On 09/29/14 06:25, Stefano Stabellini wrote:
> > >>> On Mon, 29 Sep 2014, Stefano Stabellini wrote:
> >  On Fri, 26 Sep 2014, Don Slutz wrote:
> > > This adds synchronisation of the vcpu registers
> > > between Xen and QEMU.
> > >
> > > Signed-off-by: Don Slutz 
> >  [...]
> > 
> > > diff --git a/xen-hvm.c b/xen-hvm.c
> > > index 05e522c..e1274bb 100644
> > > --- a/xen-hvm.c
> > > +++ b/xen-hvm.c
> > > @@ -857,14 +857,48 @@ static void cpu_handle_ioreq(void
> *opaque)
> > >  handle_buffered_iopage(state);
> > >if (req) {
> > > +#ifdef IOREQ_TYPE_VMWARE_PORT
> >  Is there any reason to #ifdef this code?
> >  Couldn't we just always build it in QEMU?
> > >> This allows QEMU 2.2 (or later) to build on a system that has Xen 4.5
> > >> or earlier installed; and work.
> > > I would rather remove the #ifdef from here and add any necessary
> > > compatibility stuff to include/hw/xen/xen_common.h.
> > >
> >
> > Ok, will do.
> >
> > > +if (req->type == IOREQ_TYPE_VMWARE_PORT) {
> >  I think it would make more sense to check for
> IOREQ_TYPE_VMWARE_PORT
> >  from within handle_ioreq.
> > 
> > >> Ok, I can move it.
> > >>
> > >>
> > > +CPUX86State *env;
> > > +ioreq_t fake_req = {
> > > +.type = IOREQ_TYPE_PIO,
> > > +.addr = (uint16_t)req->size,
> > > +.size = 4,
> > > +.dir = IOREQ_READ,
> > > +.df = 0,
> > > +.data_is_ptr = 0,
> > > +};
> > >>> Why do you need a fake req?
> > >> To transport the 6 VCPU registers (only 32bits of them) that vmport.c
> > >> needs to do it's work.
> > >>
> > >>> Couldn't Xen send the real req instead?
> > >> Yes, but then a 2nd exchange between QEMU and Xen would be
> needed
> > >> to fetch the 6 VCPU registers.  The ways I know of to fetch the VCPU
> registers
> > >> from Xen, all need many cycles to do their work and return
> > >> a lot of data that is not needed.
> > >>
> > >> The other option that I have considered was to extend the ioreq_t type
> > >> to have room for these registers, but that reduces by almost half the
> > >> maximum number of VCPUs that are supported (They all live on 1 page).
> > > Urgh. Now that I understand the patch better is think it's horrible, no
> > > offense :-)
> >
> > None taken.
> >
> > > Why don't you add another new ioreq type to send out the vcpu state?
> > > Something like IOREQ_TYPE_VCPU_SYNC_REGS? You could send it to
> QEMU
> > > before IOREQ_TYPE_VMWARE_PORT. Actually this solution looks very
> imilar
> > > to Alex's suggestion.
> > >
> >
> > I can, it is just slower.  This would require 2 new types.  1 for regs to
> > QEMU, 1 for regs from QEMU.  So instead of 1 round trip (Xen to QEMU
> > to Xen), you now have 3 (Xen to QEMU (regs to QEMU) to Xen, Xen to
> > QEMU (PIO) to Xen, Xen to QEMU (regs from QEMU) to Xen).
> 
> This is not an high performance device, is it?
> 
> In any case I agree it would be better to avoid it.
> I wonder if we could send both ioreqs at once from Xen and back from
> QEMU. Or maybe append the registers to IOREQ_TYPE_VMWARE_PORT,
> changing
> the size of ioreq_t only for this ioreq type.
> Another maybe simpler possibility would be introducing a union in
> ioreq_t with the registers.
> Anything would be OK for me but I would like to see the registers being
> passes explicitely by Xen rather than "hiding" them into other ioreq
> fields.
> 

Changing the size of ioreq_t would be a bit of a nightmare: The structs are 
laid out in an array indexed by vcpu and shared by QEMU and Xen.

  Paul

> 
> > >>>If any case you should spend a
> > >>> few more words on why you are doing this.
> > >>>
> > >> Sure.  Will add some form of the above as part of the commit message.
> > >>
> > > +if (!xen_opaque_env) {
> > > +xen_opaque_env = g_malloc(sizeof(CPUX86State));
> > > +}
> > > +env = xen_opaque_env;
> > > +env->regs[R_EAX] = (uint32_t)(req->addr >> 32);
> > > +env->regs[R_EBX] = (uint32_t)(req->addr);
> > > +env->regs[R_ECX] = req->count;
> > > +en

Re: [Qemu-devel] [RFC PATCH v0 0/2] target-ppc: Fix an invalid free

[Alexander Graf]

On 26.09.14 11:07, Bharata B Rao wrote:
> g_free() in target-ppc/translate_init.c:ppc_cpu_unrealizefn() can fail
> due to invalid pointer being passed to it. Fix this along with a cleanup.
> 
> I have never seen ppc_cpu_unrealizefn() getting called for sPAPR guests,
> but I ran into this issue when I was adding unrealize call from the CPU
> hot removal path while working on CPU [un]hotplug support for sPAPR guests.

This opcode table handling code really is pretty ugly code :).

Thanks, applied both to ppc-next.


Alex

Re: [Qemu-devel] [PATCH] hw/pci/ppc4xx_pci.c: Remove unused pci4xx_cfgaddr_read/write/ops

[Alexander Graf]

On 14.09.14 21:38, Peter Maydell wrote:
> The MemoryRegionOps struct pci4xx_cfgaddr_ops and the read and
> write functions it references are all unused; remove them.
> 
> Signed-off-by: Peter Maydell 

Thanks, applied to ppc-next.


Alex

Re: [Qemu-devel] [Xen-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport

[Ian Campbell]

On Wed, 2014-10-01 at 10:20 +0100, Stefano Stabellini wrote:
> I wonder if we could send both ioreqs at once from Xen and back from
> QEMU. Or maybe append the registers to IOREQ_TYPE_VMWARE_PORT, changing
> the size of ioreq_t only for this ioreq type.

Random idea: Why new add a IOREQ_TYPE_FULL_STATE which would be issued
for these ports and let qemu decode the fact that it is vmware
internally? That might be a more generically useful interface in the
future?

WRT to fitting all the register state in the current sized request, you
could declare that this new thing takes multiple slots.

Also, I may be wrong, but I thought most IOREQs were synchronous so only
one slot was ever used? The buffered ioreq stuff has a separate ring (or
uses a different part of the page, or something). I might be talking
nonsense here though ;-)

Ian.

Re: [Qemu-devel] [PATCH v5 05/33] target-arm: make arm_current_pl() return PL3

[Greg Bellows]

Yes, good catch.  Fixed in next version.

On 30 September 2014 20:23, Sergey Fedorov  wrote:

> On 30.09.2014 14:49, Greg Bellows wrote:
> > From: Fabian Aggeler 
> >
> > Make arm_current_pl() return PL3 for secure PL1 and monitor mode.
> > Increase MMU modes since mmu_index is directly infered from arm_
> > current_pl(). Changes assertion in arm_el_is_aa64() to allow EL3.
>
> arm_current_pl() is renamed in previous patch :)
>
> >
> > Signed-off-by: Fabian Aggeler 
> > Signed-off-by: Greg Bellows 
>
>

Re: [Qemu-devel] [PATCH v5 31/33] target-arm: make c13 cp regs banked (FCSEIDR, ...)

[Greg Bellows]

I have fixed-up some of the bank definitions and names so they more
accurately match the ARMv8 mappings.  In next version.

On 30 September 2014 16:49, Greg Bellows  wrote:

> From: Fabian Aggeler 
>
> When EL3 is running in Aarch32 (or ARMv7 with Security Extensions)
> FCSEIDR, CONTEXTIDR, TPIDRURW, TPIDRURO and TPIDRPRW have a secure
> and a non-secure instance.
>
> Signed-off-by: Fabian Aggeler 
> Signed-off-by: Greg Bellows 
>
> --
> v3 -> v4
> - Fix tpidrprw mapping
> ---
>  target-arm/cpu.h| 45 -
>  target-arm/helper.c | 36 
>  2 files changed, 64 insertions(+), 17 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 7a8eaef..7d27c69 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -322,11 +322,46 @@ typedef struct CPUARMState {
>  uint64_t vbar_el[4];
>  };
>  uint64_t mvbar; /* (monitor) vector base address register */
> -uint32_t c13_fcse; /* FCSE PID.  */
> -uint64_t contextidr_el1; /* Context ID.  */
> -uint64_t tpidr_el0; /* User RW Thread register.  */
> -uint64_t tpidrro_el0; /* User RO Thread register.  */
> -uint64_t tpidr_el1; /* Privileged Thread register.  */
> +struct { /* FCSE PID. */
> +uint32_t c13_fcseidr_ns;
> +uint32_t c13_fcseidr_s;
> +};
> +union { /* Context ID. */
> +struct {
> +uint64_t contextidr_ns;
> +uint64_t contextidr_s;
> +};
> +struct {
> +uint64_t contextidr_el1;
> +};
> +};
> +union { /* User RW Thread register. */
> +struct {
> +uint64_t tpidrurw_ns;
> +uint64_t tpidrurw_s;
> +};
> +struct {
> +uint64_t tpidr_el0;
> +};
> +};
> +union { /* User RO Thread register. */
> +struct {
> +uint64_t tpidruro_ns;
> +uint64_t tpidruro_s;
> +};
> +struct {
> +uint64_t tpidrro_el0;
> +};
> +};
> +union { /* Privileged Thread register. */
> +struct {
> +uint64_t tpidrprw_ns;
> +uint64_t tpidrprw_s;
> +};
> +struct {
> +uint64_t tpidr_el1;
> +};
> +};
>  uint64_t c14_cntfrq; /* Counter Frequency register */
>  uint64_t c14_cntkctl; /* Timer Control register */
>  ARMGenericTimer c14_timer[NUM_GTIMERS];
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e91a019..8d9563f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -420,12 +420,15 @@ static void tlbimvaa_is_write(CPUARMState *env,
> const ARMCPRegInfo *ri,
>
>  static const ARMCPRegInfo cp_reginfo[] = {
>  { .name = "FCSEIDR", .cp = 15, .crn = 13, .crm = 0, .opc1 = 0, .opc2
> = 0,
> -  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState,
> cp15.c13_fcse),
> +  .access = PL1_RW,
> +  .bank_fieldoffsets = { offsetof(CPUARMState, cp15.c13_fcseidr_s),
> + offsetof(CPUARMState, cp15.c13_fcseidr_ns) },
>.resetvalue = 0, .writefn = fcse_write, .raw_writefn = raw_write, },
> -{ .name = "CONTEXTIDR", .state = ARM_CP_STATE_BOTH,
> -  .opc0 = 3, .opc1 = 0, .crn = 13, .crm = 0, .opc2 = 1,
> +{ .name = "CONTEXTIDR",
> +  .cp = 15, .opc1 = 0, .crn = 13, .crm = 0, .opc2 = 1,
>.access = PL1_RW,
> -  .fieldoffset = offsetof(CPUARMState, cp15.contextidr_el1),
> +  .bank_fieldoffsets = { offsetof(CPUARMState, cp15.contextidr_s),
> + offsetof(CPUARMState, cp15.contextidr_ns) },
>.resetvalue = 0, .writefn = contextidr_write, .raw_writefn =
> raw_write, },
>  REGINFO_SENTINEL
>  };
> @@ -1042,21 +1045,25 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
>.access = PL0_RW,
>.fieldoffset = offsetof(CPUARMState, cp15.tpidr_el0), .resetvalue =
> 0 },
>  { .name = "TPIDRURW", .cp = 15, .crn = 13, .crm = 0, .opc1 = 0, .opc2
> = 2,
> -  .access = PL0_RW,
> -  .fieldoffset = offsetoflow32(CPUARMState, cp15.tpidr_el0),
> -  .resetfn = arm_cp_reset_ignore },
> +  .access = PL0_RW, .resetvalue = 0,
> +  .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.tpidrurw_s),
> + offsetoflow32(CPUARMState, cp15.tpidrurw_ns)
> } },
>  { .name = "TPIDRRO_EL0", .state = ARM_CP_STATE_AA64,
>.opc0 = 3, .opc1 = 3, .opc2 = 3, .crn = 13, .crm = 0,
>.access = PL0_R|PL1_W,
>.fieldoffset = offsetof(CPUARMState, cp15.tpidrro_el0), .resetvalue
> = 0 },
>  { .name = "TPIDRURO", .cp = 15, .crn = 13, .crm = 0, .opc1 = 0, .opc2
> = 3,
> -  .access = PL0_R|PL1_W,
> -  .fieldoffset = offsetoflow32(CPUARMState, cp15.tpidrro_el0

Re: [Qemu-devel] [PATCH 04/12] spapr_pci: add set-indicator RTAS interface

[Alexander Graf]

On 01.10.14 00:08, Michael Roth wrote:
> Quoting Alexander Graf (2014-08-26 06:36:57)
>> On 19.08.14 02:21, Michael Roth wrote:
>>> From: Mike Day 
>>>
>>> Signed-off-by: Mike Day 
>>> Signed-off-by: Michael Roth 
>>> ---
>>>  hw/ppc/spapr_pci.c | 119 
>>> +
>>>  include/hw/ppc/spapr.h |   3 ++
>>>  2 files changed, 122 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index 924d488..23a3477 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -36,6 +36,16 @@
>>>  
>>>  #include "hw/pci/pci_bus.h"
>>>  
>>> +/* #define DEBUG_SPAPR */
>>> +
>>> +#ifdef DEBUG_SPAPR
>>> +#define DPRINTF(fmt, ...) \
>>> +do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
>>> +#else
>>> +#define DPRINTF(fmt, ...) \
>>> +do { } while (0)
>>> +#endif
>>> +
>>>  /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */
>>>  #define RTAS_QUERY_FN   0
>>>  #define RTAS_CHANGE_FN  1
>>> @@ -47,6 +57,31 @@
>>>  #define RTAS_TYPE_MSI   1
>>>  #define RTAS_TYPE_MSIX  2
>>>  
>>> +/* For set-indicator RTAS interface */
>>> +#define INDICATOR_ISOLATION_MASK0x0001   /* 9001 one bit */
>>> +#define INDICATOR_GLOBAL_INTERRUPT_MASK 0x0002   /* 9005 one bit */
>>> +#define INDICATOR_ERROR_LOG_MASK0x0004   /* 9006 one bit */
>>> +#define INDICATOR_IDENTIFY_MASK 0x0008   /* 9007 one bit */
>>> +#define INDICATOR_RESET_MASK0x0010   /* 9009 one bit */
>>> +#define INDICATOR_DR_MASK   0x00e0   /* 9002 three bits */
>>> +#define INDICATOR_ALLOCATION_MASK   0x0300   /* 9003 two bits */
>>> +#define INDICATOR_EPOW_MASK 0x1c00   /* 9 three bits */
>>> +
>>> +#define INDICATOR_ISOLATION_SHIFT   0x00 /* bit 0 */
>>> +#define INDICATOR_GLOBAL_INTERRUPT_SHIFT0x01 /* bit 1 */
>>> +#define INDICATOR_ERROR_LOG_SHIFT   0x02 /* bit 2 */
>>> +#define INDICATOR_IDENTIFY_SHIFT0x03 /* bit 3 */
>>> +#define INDICATOR_RESET_SHIFT   0x04 /* bit 4 */
>>> +#define INDICATOR_DR_SHIFT  0x05 /* bits 5-7 */
>>> +#define INDICATOR_ALLOCATION_SHIFT  0x08 /* bits 8-9 */
>>> +#define INDICATOR_EPOW_SHIFT0x0a /* bits 10-12 */
>>> +
>>> +#define DECODE_DRC_STATE(state, m, s)  \
>>> +uint32_t)(state) & (uint32_t)(m))) >> (s))
>>> +
>>> +#define ENCODE_DRC_STATE(val, m, s) \
>>> +(((uint32_t)(val) << (s)) & (uint32_t)(m))
>>> +
>>>  static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid)
>>>  {
>>>  sPAPRPHBState *sphb;
>>> @@ -402,6 +437,80 @@ static void 
>>> rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
>>>  rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
>>>  }
>>>  
>>> +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>> +   uint32_t token, uint32_t nargs,
>>> +   target_ulong args, uint32_t nret,
>>> +   target_ulong rets)
>>> +{
>>> +uint32_t indicator = rtas_ld(args, 0);
>>> +uint32_t drc_index = rtas_ld(args, 1);
>>> +uint32_t indicator_state = rtas_ld(args, 2);
>>> +uint32_t encoded = 0, shift = 0, mask = 0;
>>> +uint32_t *pind;
>>> +sPAPRDrcEntry *drc_entry = NULL;
>>
>> This rtas call does not have any idea what a PHB is. Why does it live in
>> spapr_pci.c?
> 
> spapr_rtas.c does seem like a better home
> 
>>
>>> +
>>> +if (drc_index == 0) { /* platform indicator */
>>> +pind = &spapr->state;
>>> +} else {
>>> +drc_entry = spapr_find_drc_entry(drc_index);
>>> +if (!drc_entry) {
>>> +DPRINTF("rtas_set_indicator: unable to find drc_entry for %x",
>>> +drc_index);
>>> +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>> +return;
>>> +}
>>> +pind = &drc_entry->state;
>>> +}
>>> +
>>> +switch (indicator) {
>>> +case 9:  /* EPOW */
>>> +shift = INDICATOR_EPOW_SHIFT;
>>> +mask = INDICATOR_EPOW_MASK;
>>> +break;
>>> +case 9001: /* Isolation state */
>>> +/* encode the new value into the correct bit field */
>>> +shift = INDICATOR_ISOLATION_SHIFT;
>>> +mask = INDICATOR_ISOLATION_MASK;
>>> +break;
>>> +case 9002: /* DR */
>>> +shift = INDICATOR_DR_SHIFT;
>>> +mask = INDICATOR_DR_MASK;
>>> +break;
>>> +case 9003: /* Allocation State */
>>> +shift = INDICATOR_ALLOCATION_SHIFT;
>>> +mask = INDICATOR_ALLOCATION_MASK;
>>> +break;
>>> +case 9005: /* global interrupt */
>>> +shift = INDICATOR_GLOBAL_INTERRUPT_SHIFT;
>>> +mask = INDICATOR_GLOBAL_INTERRUPT_MASK;
>>> +break;
>>> +case 9006: /* error log */
>>> +shift = INDICATOR_ERROR_LOG_SHIFT;
>>> +mask = INDICATOR_ERROR

Re: [Qemu-devel] hid keyboard event handling

[Michael Walle]

Am 2014-10-01 16:17, schrieb Gerd Hoffmann:

Hi,

My input device model (milkymist-softusb.c) polls 
(hid_keyboard_poll())
exactly once per event callback. Actually, i don't see any other way 
to

do it because the hid_keyboard_poll() always succeeds even if there is
no event in the queue.


You can use hid_has_events() to figure whenever there are events in the
queue or not.


Oh, didn't see that function. Thanks, that should do the trick.

-michael

[Qemu-devel] [PATCH 3/3] PPC: E500: Hook up power off GPIO to GPIO controller

[Alexander Graf]

Now that we have a working GPIO controller on the virt machine, we can use
one pin to notify QEMU that the guests wants to power off the system.

Signed-off-by: Alexander Graf 
---
 hw/ppc/e500.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index db82f72..cc12f90 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -177,6 +177,8 @@ static void create_dt_mpc8xxx_gpio(void *fdt, const char 
*soc, const char *mpic)
 hwaddr mmio0 = MPC8XXX_GPIO_OFFSET;
 int irq0 = MPC8XXX_GPIO_IRQ;
 gchar *node = g_strdup_printf("%s/gpio@%"PRIx64, soc, mmio0);
+gchar *poweroff = g_strdup_printf("%s/power-off", soc);
+int gpio_ph;
 
 qemu_fdt_add_subnode(fdt, node);
 qemu_fdt_setprop_string(fdt, node, "compatible", "fsl,qoriq-gpio");
@@ -185,8 +187,17 @@ static void create_dt_mpc8xxx_gpio(void *fdt, const char 
*soc, const char *mpic)
 qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
 qemu_fdt_setprop_cells(fdt, node, "#gpio-cells", 2);
 qemu_fdt_setprop(fdt, node, "gpio-controller", NULL, 0);
+gpio_ph = qemu_fdt_alloc_phandle(fdt);
+qemu_fdt_setprop_cell(fdt, node, "phandle", gpio_ph);
+qemu_fdt_setprop_cell(fdt, node, "linux,phandle", gpio_ph);
+
+/* Power Off Pin */
+qemu_fdt_add_subnode(fdt, poweroff);
+qemu_fdt_setprop_string(fdt, poweroff, "compatible", "gpio-poweroff");
+qemu_fdt_setprop_cells(fdt, poweroff, "gpios", gpio_ph, 0, 0);
 
 g_free(node);
+g_free(poweroff);
 }
 
 static int sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
@@ -765,6 +776,13 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, 
MemoryRegion *ccsr,
 return mpic;
 }
 
+static void ppce500_power_off(void *opaque, int line, int on)
+{
+if (on) {
+qemu_system_shutdown_request();
+}
+}
+
 void ppce500_init(MachineState *machine, PPCE500Params *params)
 {
 MemoryRegion *address_space_mem = get_system_memory();
@@ -936,12 +954,18 @@ void ppce500_init(MachineState *machine, PPCE500Params 
*params)
 }
 
 if (params->has_mpc8xxx_gpio) {
+qemu_irq poweroff_irq;
+
 dev = qdev_create(NULL, "mpc8xxx_gpio");
 s = SYS_BUS_DEVICE(dev);
 qdev_init_nofail(dev);
 sysbus_connect_irq(s, 0, mpic[MPC8XXX_GPIO_IRQ]);
 memory_region_add_subregion(ccsr_addr_space, MPC8XXX_GPIO_OFFSET,
 sysbus_mmio_get_region(s, 0));
+
+/* Power Off GPIO at Pin 0 */
+poweroff_irq = qemu_allocate_irq(ppce500_power_off, NULL, 0);
+qdev_connect_gpio_out(dev, 0, poweroff_irq);
 }
 
 /* Load kernel. */
-- 
1.8.1.4

Re: [Qemu-devel] hid keyboard event handling

[Gerd Hoffmann]

  Hi,

> My input device model (milkymist-softusb.c) polls (hid_keyboard_poll()) 
> exactly once per event callback. Actually, i don't see any other way to 
> do it because the hid_keyboard_poll() always succeeds even if there is 
> no event in the queue.

You can use hid_has_events() to figure whenever there are events in the
queue or not.

cheers,
  Gerd

[Qemu-devel] [PATCH 2/3] PPC: E500: Instantiate MPC8XXX gpio controller on virt machine

[Alexander Graf]

With the e500 virt machine, we don't have to adhere to the exact hardware
layout of an mpc8544ds board. So there we can just add a qoriq compatible
GPIO controller into the system that we can add a power off hook to.

Signed-off-by: Alexander Graf 
---
 hw/ppc/e500.c | 32 
 hw/ppc/e500.h |  1 +
 hw/ppc/e500plat.c |  1 +
 3 files changed, 34 insertions(+)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 22cfeac..db82f72 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -64,6 +64,8 @@
 #define MPC8544_PCI_IO 0xE100ULL
 #define MPC8544_UTIL_OFFSET0xeULL
 #define MPC8544_SPIN_BASE  0xEF00ULL
+#define MPC8XXX_GPIO_OFFSET0x000FF000ULL
+#define MPC8XXX_GPIO_IRQ   43
 
 struct boot_info
 {
@@ -170,6 +172,23 @@ static int create_devtree_etsec(SysBusDevice *sbdev, 
PlatformDevtreeData *data)
 return 0;
 }
 
+static void create_dt_mpc8xxx_gpio(void *fdt, const char *soc, const char 
*mpic)
+{
+hwaddr mmio0 = MPC8XXX_GPIO_OFFSET;
+int irq0 = MPC8XXX_GPIO_IRQ;
+gchar *node = g_strdup_printf("%s/gpio@%"PRIx64, soc, mmio0);
+
+qemu_fdt_add_subnode(fdt, node);
+qemu_fdt_setprop_string(fdt, node, "compatible", "fsl,qoriq-gpio");
+qemu_fdt_setprop_cells(fdt, node, "reg", mmio0, 0x1000);
+qemu_fdt_setprop_cells(fdt, node, "interrupts", irq0, 0x2);
+qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
+qemu_fdt_setprop_cells(fdt, node, "#gpio-cells", 2);
+qemu_fdt_setprop(fdt, node, "gpio-controller", NULL, 0);
+
+g_free(node);
+}
+
 static int sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
 {
 PlatformDevtreeData *data = opaque;
@@ -490,6 +509,10 @@ static int ppce500_load_device_tree(MachineState *machine,
 qemu_fdt_setprop_cell(fdt, pci, "#address-cells", 3);
 qemu_fdt_setprop_string(fdt, "/aliases", "pci0", pci);
 
+if (params->has_mpc8xxx_gpio) {
+create_dt_mpc8xxx_gpio(fdt, soc, mpic);
+}
+
 if (params->has_platform_bus) {
 platform_bus_create_devtree(params, fdt, mpic);
 }
@@ -912,6 +935,15 @@ void ppce500_init(MachineState *machine, PPCE500Params 
*params)
 sysbus_mmio_get_region(s, 0));
 }
 
+if (params->has_mpc8xxx_gpio) {
+dev = qdev_create(NULL, "mpc8xxx_gpio");
+s = SYS_BUS_DEVICE(dev);
+qdev_init_nofail(dev);
+sysbus_connect_irq(s, 0, mpic[MPC8XXX_GPIO_IRQ]);
+memory_region_add_subregion(ccsr_addr_space, MPC8XXX_GPIO_OFFSET,
+sysbus_mmio_get_region(s, 0));
+}
+
 /* Load kernel. */
 if (machine->kernel_filename) {
 kernel_base = cur_base;
diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index f1b2766..08a4c3d 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -16,6 +16,7 @@ typedef struct PPCE500Params {
 hwaddr platform_bus_size;
 int platform_bus_first_irq;
 int platform_bus_num_irqs;
+bool has_mpc8xxx_gpio;
 } PPCE500Params;
 
 void ppce500_init(MachineState *machine, PPCE500Params *params);
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index befe1d1..2d14d58 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -40,6 +40,7 @@ static void e500plat_init(MachineState *machine)
 .platform_bus_size = (128ULL * 1024 * 1024),
 .platform_bus_first_irq = 5,
 .platform_bus_num_irqs = 10,
+.has_mpc8xxx_gpio = true,
 };
 
 /* Older KVM versions don't support EPR which breaks guests when we 
announce
-- 
1.8.1.4

[Qemu-devel] [PATCH 1/3] PPC: Add MPC8XXX gpio controller

[Alexander Graf]

On e500 systems most SoCs implement a common GPIO controller that Linux
calls the "mpc8xxx" gpio controller. This patch adds an emulation model
for this device.

Signed-off-by: Alexander Graf 
---
 hw/gpio/Makefile.objs |   1 +
 hw/gpio/mpc8xxx.c | 217 ++
 2 files changed, 218 insertions(+)
 create mode 100644 hw/gpio/mpc8xxx.c

diff --git a/hw/gpio/Makefile.objs b/hw/gpio/Makefile.objs
index 2c8b51f..1abcf17 100644
--- a/hw/gpio/Makefile.objs
+++ b/hw/gpio/Makefile.objs
@@ -2,5 +2,6 @@ common-obj-$(CONFIG_MAX7310) += max7310.o
 common-obj-$(CONFIG_PL061) += pl061.o
 common-obj-$(CONFIG_PUV3) += puv3_gpio.o
 common-obj-$(CONFIG_ZAURUS) += zaurus.o
+common-obj-$(CONFIG_E500) += mpc8xxx.o
 
 obj-$(CONFIG_OMAP) += omap_gpio.o
diff --git a/hw/gpio/mpc8xxx.c b/hw/gpio/mpc8xxx.c
new file mode 100644
index 000..1ae
--- /dev/null
+++ b/hw/gpio/mpc8xxx.c
@@ -0,0 +1,217 @@
+/*
+ *  GPIO Controller for a lot of Freescale SoCs
+ *
+ * Copyright (C) 2014 Freescale Semiconductor, Inc. All rights reserved.
+ *
+ * Author: Alexander Graf, 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "hw/sysbus.h"
+
+#define TYPE_MPC8XXX_GPIO "mpc8xxx_gpio"
+#define MPC8XXX_GPIO(obj) OBJECT_CHECK(MPC8XXXGPIOState, (obj), 
TYPE_MPC8XXX_GPIO)
+
+typedef struct MPC8XXXGPIOState {
+SysBusDevice parent_obj;
+
+MemoryRegion iomem;
+qemu_irq irq;
+qemu_irq out[32];
+
+uint32_t dir;
+uint32_t odr;
+uint32_t dat;
+uint32_t ier;
+uint32_t imr;
+uint32_t icr;
+} MPC8XXXGPIOState;
+
+static const VMStateDescription vmstate_mpc8xxx_gpio = {
+.name = "mpc8xxx_gpio",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(dir, MPC8XXXGPIOState),
+VMSTATE_UINT32(odr, MPC8XXXGPIOState),
+VMSTATE_UINT32(dat, MPC8XXXGPIOState),
+VMSTATE_UINT32(ier, MPC8XXXGPIOState),
+VMSTATE_UINT32(imr, MPC8XXXGPIOState),
+VMSTATE_UINT32(icr, MPC8XXXGPIOState),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static void mpc8xxx_gpio_update(MPC8XXXGPIOState *s)
+{
+qemu_set_irq(s->irq, !!(s->ier & s->imr));
+}
+
+static uint64_t mpc8xxx_gpio_read(void *opaque, hwaddr offset,
+  unsigned size)
+{
+MPC8XXXGPIOState *s = (MPC8XXXGPIOState *)opaque;
+
+if (size != 4) {
+/* All registers are 32bit */
+return 0;
+}
+
+switch (offset) {
+case 0x0: /* Direction */
+return s->dir;
+case 0x4: /* Open Drain */
+return s->odr;
+case 0x8: /* Data */
+return s->dat;
+case 0xC: /* Interrupt Event */
+return s->ier;
+case 0x10: /* Interrupt Mask */
+return s->imr;
+case 0x14: /* Interrupt Control */
+return s->icr;
+default:
+return 0;
+}
+}
+
+static void mpc8xxx_write_data(MPC8XXXGPIOState *s, uint32_t new_data)
+{
+uint32_t old_data = s->dat;
+uint32_t diff = old_data ^ new_data;
+int i;
+
+for (i = 0; i < 32; i++) {
+uint32_t mask = 0x8000 >> i;
+if (!(diff & mask)) {
+continue;
+}
+
+if (s->dir & mask) {
+/* Output */
+qemu_set_irq(s->out[i], (new_data & mask) != 0);
+}
+}
+
+s->dat = new_data;
+}
+
+static void mpc8xxx_gpio_write(void *opaque, hwaddr offset,
+uint64_t value, unsigned size)
+{
+MPC8XXXGPIOState *s = (MPC8XXXGPIOState *)opaque;
+
+if (size != 4) {
+/* All registers are 32bit */
+return;
+}
+
+switch (offset) {
+case 0x0: /* Direction */
+s->dir = value;
+break;
+case 0x4: /* Open Drain */
+s->odr = value;
+break;
+case 0x8: /* Data */
+mpc8xxx_write_data(s, value);
+break;
+case 0xC: /* Interrupt Event */
+s->ier &= ~value;
+break;
+case 0x10: /* Interrupt Mask */
+s->imr = value;
+break;
+case 0x14: /* Interrupt Control */
+s->icr = value;
+break;
+}
+
+mpc8xxx_gpio_update(s);
+}
+
+static void mpc8xxx_gpio_reset(MPC8XXXGPIOState *s)
+{
+s->dir = 0;
+s->odr = 0;
+s->dat = 0;
+s->ier = 0;
+s->imr = 0;
+s->icr = 0;
+}
+
+static void mpc8xxx_gpio_set_irq(void * opaque, int irq, i

[Qemu-devel] [PATCH 0/3] PPC: E500: Add power off GPIO

[Alexander Graf]

When running a virtual machine, you eventually want to shut it down and see it
properly end the QEMU process. The magic that makes this work is the "power off"
code path that a guest OS uses to indicate that it wants to power off the host
machine.

This logic was missing from our e500 virt machine. The easiest way to implement
this is to expose a GPIO to the guest that allows the virtual machine to
indicate to QEMU that it wants to power down now.

To expose this GPIO to the guest, we need to add a GPIO controller, the GPIO
hookup and some description in device tree. This is what this patch set does.

We only support this for the e500 virt machine though, as the mpc8544ds does
not support power off from an OS.

To make use of this logic, you will also need patches to Linux that enable it
to register the gpio-poweroff driver:

  https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-October/121532.html

And you need to make sure that the following options are enabled in your guest
kernel:

  CONFIG_POWER_SUPPLY=y
  CONFIG_POWER_RESET=y
  CONFIG_POWER_RESET_GPIO=y

Alexander Graf (3):
  PPC: Add MPC8XXX gpio controller
  PPC: E500: Instantiate MPC8XXX gpio controller on virt machine
  PPC: E500: Hook up power off GPIO to GPIO controller

 hw/gpio/Makefile.objs |   1 +
 hw/gpio/mpc8xxx.c | 217 ++
 hw/ppc/e500.c |  56 +
 hw/ppc/e500.h |   1 +
 hw/ppc/e500plat.c |   1 +
 5 files changed, 276 insertions(+)
 create mode 100644 hw/gpio/mpc8xxx.c

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v4 2/3] pcie: add check for ari capability of pcie devices

[Marcel Apfelbaum]

On Wed, 2014-10-01 at 07:26 +0200, Knut Omang wrote:
> On Tue, 2014-09-30 at 21:38 +0800, Gonglei wrote:
> > > Subject: Re: [Qemu-devel] [PATCH v4 2/3] pcie: add check for ari 
> > > capability of
> > > pcie devices
> > > 
> > > On Tue, Sep 30, 2014 at 06:11:25PM +0800, arei.gong...@huawei.com wrote:
> > > > From: Gonglei 
> > > >
> > > > In QEMU, ARI Forwarding is enabled default at emulation of PCIe
> > > > ports. ARI Forwarding enable setting at firmware/OS Control handoff.
> > > > If the bit is Set when a non-ARI Device is present, the non-ARI
> > > > Device can respond to Configuration Space accesses under what it
> > > > interprets as being different Device Numbers, and its Functions can
> > > > be aliased under multiple Device Numbers, generally leading to
> > > > undesired behavior.
> > > 
> > > So what is the undesired behaviour?
> > > Did you observe such?
> > 
> > I just observe the PCI device don't work, and the dmesg show me:
> > 
> > [ 159.035250] pciehp :05:00.0:pcie24: Button pressed on Slot (0 - 4)
> > [ 159.035274] pciehp :05:00.0:pcie24: Card present on Slot (0 - 4)
> > [ 159.036517] pciehp :05:00.0:pcie24: PCI slot #0 - 4 - powering on due 
> > to button press.
> > [ 159.188049] pciehp :05:00.0:pcie24: Failed to check link status
> > [ 159.201968] pciehp :05:00.0:pcie24: Card not present on Slot (0 - 4)
> > [ 159.202529] pciehp :05:00.0:pcie24: Already disabled on Slot (0 - 4)
> 
> This seems very much like the symptoms I see when I use hotplug after
> the latest changes to the hotplug code - do you have something that
> confirms this has something to do with wrong interpretation of device
> IDs? My suspicion is that something has gone wrong or is missing in the
> hotplug logic but I havent had time to dig deeper into it yet.
Can you please describe me the steps to reproduce the issue?
It should work correctly by now, maybe a "surprise removal/addition"
warning and nothing more.

Thank you,
Marcel

> 
> I am just concerned that this might alleviate the symptoms you see but
> not fix the problem itself,
> 
> Knut
> 
> > > Please make this explicit in the commit log.
> > > 
> > Sorry, I copied the description from PCIe spec. :(
> > 
> > IMPLEMENTATION NOTE at Page 19:
> > 
> > https://www.pcisig.com/specifications/pciexpress/specifications/ECN-alt-rid-interpretation-070604.pdf
> > 
> > 
> > > 
> > > >
> > > > So, for PCI devices attached in PCIe root ports or downstream pots,
> > > > we should assure that its slot is not non-zero. For PCIe devices, which
> > > > ARP capability is not enabled, we also should assure that its slot
> > > > is not non-zero.
> > > 
> > > not non zero => zero
> > > 
> > OK.
> > 
> > > >
> > > > Signed-off-by: Gonglei 
> > > > ---
> > > >  hw/pci/pci.c  |  4 
> > > >  hw/pci/pcie.c | 51
> > > +++
> > > >  include/hw/pci/pcie.h |  1 +
> > > >  3 files changed, 56 insertions(+)
> > > >
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index 6ce75aa..22b7ca0 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -1770,6 +1770,10 @@ static int pci_qdev_init(DeviceState *qdev)
> > > >  }
> > > >  }
> > > >
> > > > +if (pcie_cap_slot_check(bus, pci_dev)) {
> > > > +return -1;
> > > > +}
> > > > +
> > > >  /* rom loading */
> > > >  is_default_rom = false;
> > > >  if (pci_dev->romfile == NULL && pc->romfile != NULL) {
> > > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > > index 1babddf..b82211a 100644
> > > > --- a/hw/pci/pcie.c
> > > > +++ b/hw/pci/pcie.c
> > > > @@ -633,3 +633,54 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset,
> > > uint16_t nextfn)
> > > >  offset, PCI_ARI_SIZEOF);
> > > >  pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) 
> > > > << 8);
> > > >  }
> > > > +
> > > > +int pcie_cap_slot_check(PCIBus *bus, PCIDevice *dev)
> > > > +{
> > > > +Object *obj = OBJECT(bus);
> > > > +
> > > > +if (pci_bus_is_root(bus)) {
> > > > +return 0;
> > > > +}
> > > > +
> > > > +if (object_dynamic_cast(obj, TYPE_PCIE_BUS)) {
> > > > +DeviceState *parent = qbus_get_parent(BUS(obj));
> > > > +PCIDevice *pci_dev = PCI_DEVICE(parent);
> > > > +uint8_t port_type;
> > > > +/*
> > > > + * Root ports and downstream ports of switches are the hot
> > > > + * pluggable ports in a PCI Express hierarchy.
> > > > + * PCI Express supports chip-to-chip interconnect, a PCIe link 
> > > > can
> > > > + * only connect one PCI device/Switch/EndPoint or PCI-bridge.
> > > > + *
> > > > + * 7.3. Configuration Transaction Rules (PCI Express 
> > > > specification
> > > 3.0)
> > > > + * 7.3.1. Device Number
> > > > + *
> > > > + * Downstream Ports that do not have ARI Forwarding enabled
> > > must
> > > > + * associate only Device 0 with the device atta