Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely

2012-07-09 Thread Kevin Wolf
Am 15.06.2012 22:47, schrieb Supriya Kannery:
> Struct BDRVReopenState along with three reopen related functions
> introduced for handling reopening of images safely. This can be
> extended by each of the block drivers to reopen respective
> image files.
> 
> Signed-off-by: Supriya Kannery 
> 
> Index: qemu/block.c
> ===
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,10 +858,32 @@ unlink_and_fail:
>  return ret;
>  }
>  
> +int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int 
> flags)
> +{
> + BlockDriver *drv = bs->drv;
> +
> + return drv->bdrv_reopen_prepare(bs, prs, flags);
> +}
> +
> +void bdrv_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +drv->bdrv_reopen_commit(bs, rs);
> +}
> +
> +void bdrv_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +drv->bdrv_reopen_abort(bs, rs);
> +}
> +
>  int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
>  {
>  BlockDriver *drv = bs->drv;
>  int ret = 0, open_flags;
> +BDRVReopenState *reopen_state = NULL;
>  
>  /* Quiesce IO for the given block device */
>  qemu_aio_flush();
> @@ -870,17 +892,44 @@ int bdrv_reopen(BlockDriverState *bs, in
>  qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
>  return ret;
>  }
> -open_flags = bs->open_flags;
> -bdrv_close(bs);
>  
> -ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> -if (ret < 0) {
> -/* Reopen failed. Try to open with original flags */
> -qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> -ret = bdrv_open(bs, bs->filename, open_flags, drv);
> -if (ret < 0) {
> -/* Reopen failed with orig and modified flags */
> -abort();
> +/* Use driver specific reopen() if available */
> +if (drv->bdrv_reopen_prepare) {
> +ret = bdrv_reopen_prepare(bs, &reopen_state, bdrv_flags);
> + if (ret < 0) {
> +bdrv_reopen_abort(bs, reopen_state);
> +qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +return ret;
> +}
> +
> +bdrv_reopen_commit(bs, reopen_state);
> +bs->open_flags = bdrv_flags;
> +
> +} else {
> +
> +/* Try reopening the image using protocol or directly */
> +if (bs->file) {
> +open_flags = bs->open_flags;
> +drv->bdrv_close(bs);
> +if (drv->bdrv_file_open) {
> +ret = drv->bdrv_file_open(bs, bs->filename, bdrv_flags);
> +} else {
> +ret = bdrv_file_open(&bs->file, bs->filename, bdrv_flags);

Doesn't this forget to reopen bs itself? And bs->file wasn't even
closed. If think we need something more along the lines of:

if (bs->file) {
bdrv_reopen(bs->file)
}

if (drv->bdrv_file_open) {
drv->bdrv_file_open(bs)
} else {
   drv->bdrv_open(bs)
}

In fact we would really want to be able to commit/abort the bdrv_reopen
of bs->file only after we know if bdrv_open(bs) has succeeded, but with
the current design we can't because bdrv_open wants to read from the new fd.

Maybe it would make sense to require bdrv_reopen_prepare() to do the
switch without throwing the old state away yet, but it sounds as if it
would make implementations for image formats quite a bit harder.

Maybe we should completely avoid this default implementation and just
fail bdrv_reopen if a driver doesn't support the explicit,
transactionable reopen functions.

Kevin



Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd

2012-07-09 Thread Corey Bryant



On 07/09/2012 10:05 AM, Luiz Capitulino wrote:

On Thu, 05 Jul 2012 11:06:56 -0400
Corey Bryant  wrote:




On 07/04/2012 04:09 AM, Kevin Wolf wrote:

Am 03.07.2012 20:21, schrieb Corey Bryant:

On 07/03/2012 02:00 PM, Eric Blake wrote:

On 07/03/2012 11:46 AM, Corey Bryant wrote:



Yes, I think adding a +1 to the refcount for the monitor makes sense.

I'm a bit unsure how to increment the refcount when a monitor reconnects
though.  Maybe it is as simple as adding a +1 to each fd's refcount when
the next QMP monitor connects.


Or maybe delay a +1 until after a 'query-fds' - it is not until the
monitor has reconnected and learned what fds it should be aware of that
incrementing the refcount again makes sense.  But that would mean making
'query-fds' track whether this is the first call since the monitor
reconnected, as it shouldn't normally increase refcounts.


This doesn't sound ideal.


Yes, it's less than ideal.


The other alternative is that the monitor never re-increments a
refcount.  Once a monitor disconnects, that fd is lost to the monitor,
and a reconnected monitor must pass in a new fd to be re-associated with
the fdset.  In other words, the monitor's use of an fd is a one-way
operation, starting life in use but ending at the first disconnect or
remove-fd.


I would vote for this 2nd alternative.  As long as we're not introducing
an fd leak.  And I don't think we are if we decrement the refcount on
remove-fd or on QMP disconnect.


In fact, I believe this one is even worse. I can already see hacks like
adding a dummy FD with invalid flags and removing it again just to
regain control over the fdset...

You earlier suggestion made a lot of sense to me: Whenever a new QMP
monitor is connected, increase the refcount. That is, as long as any
monitor is there, don't drop any fdsets unless explicitly requested via QMP.


Ok.  So refcount would be incremented (for the fd or fdset, whatever we
decide on) when QMP reconnects.  I'm assuming we wouldn't wait until
after a query-fds call.


I'm not sure this is a good idea because we will leak fds if the client forgets
about the fds when re-connecting (ie. it was restarted) or if a different
client connects to QMP.

If we really want to do that, I think that the right way of doing this is to
add a command for clients to re-again ownership of the fds on reconnection.

But to be honest, I don't fully understand why this is needed.



I'm not sure this is an issue with current design.  I know things have 
changed a bit as the email threads evolved, so I'll paste the current 
design that I am working from.  Please let me know if you still see any 
issues.


FD passing:
---
New monitor commands enable adding/removing an fd to/from a set.  New 
monitor command query-fdsets enables querying of current monitor fdsets. 
 The set of fds should all refer to the same file, with each fd having 
different access flags (ie. O_RDWR, O_RDONLY).  qemu_open can then dup 
the fd that has the matching access mode flags.


Design points:
--
1. add-fd
-> fd is passed via SCM rights and qemu adds fd to first unused fdset 
(e.g. /dev/fdset/1)
-> add-fd monitor function initializes the monitor inuse flag for the 
fdset to true

-> add-fd monitor function initializes the remove flag for the fd to false
-> add-fd returns fdset number and received fd number (e.g fd=3) to caller

2. drive_add file=/dev/fdset/1
-> qemu_open uses the first fd in fdset1 that has access flags matching 
the qemu_open action flags and has remove flag set to false

-> qemu_open increments refcount for the fdset
-> Need to make sure that if a command like 'device-add' fails that 
refcount is not incremented


3. add-fd fdset=1
-> fd is passed via SCM rights
-> add-fd monitor function adds the received fd to the specified fdset 
(or fails if fdset doesn't exist)

-> add-fd monitor function initializes the remove flag for the fd to false
-> add-fd returns fdset number and received fd number (e.g fd=4) to caller

4. block-commit
-> qemu_open performs "reopen" by using the first fd from the fdset that 
has access flags matching the qemu_open action flags and has remove flag 
set to false

-> qemu_open increments refcount for the fdset
-> Need to make sure that if a command like 'block-commit' fails that 
refcount is not incremented


5. remove-fd fdset=1 fd=4
-> remove-fd monitor function fails if fdset doesn't exist
-> remove-fd monitor function turns on remove flag for fd=4

6. qemu_close (need to replace all close calls in block layer with 
qemu_close)

-> qemu_close decrements refcount for fdset
-> qemu_close closes all fds that have (refcount == 0 && (!inuse || remove))
-> qemu_close frees the fdset if no fds remain in it

7. disconnecting the QMP monitor
-> monitor disconnect visits all fdsets on monitor and turns off monitor 
in-use flag for fdset


8. connecting the QMP monitor
-> monitor connect visits all fdsets on monitor and turns on monitor 
in-use flag for fdset


9. query-fdset

[Qemu-devel] [PATCH 20/25] fdc_test: update media_change test

2012-07-09 Thread Kevin Wolf
From: Pavel Hrdina 

After rewrite DSKCHG bit handling the test has to be updated. Now
is needed to seek to different track to clear DSKCHG bit.

Signed-off-by: Pavel Hrdina 
Signed-off-by: Kevin Wolf 
---
 tests/fdc-test.c |   25 +
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 610e2f1..5f52f6c 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -156,19 +156,16 @@ static uint8_t send_read_command(void)
 return ret;
 }
 
-static void send_step_pulse(void)
+static void send_step_pulse(int cyl)
 {
 int drive = 0;
 int head = 0;
-static int cyl = 0;
 
 floppy_send(CMD_SEEK);
 floppy_send(head << 2 | drive);
 g_assert(!get_irq(FLOPPY_IRQ));
 floppy_send(cyl);
 ack_irq();
-
-cyl = (cyl + 1) % 4;
 }
 
 static uint8_t cmos_read(uint8_t reg)
@@ -195,8 +192,7 @@ static void test_no_media_on_start(void)
 assert_bit_set(dir, DSKCHG);
 dir = inb(FLOPPY_BASE + reg_dir);
 assert_bit_set(dir, DSKCHG);
-send_step_pulse();
-send_step_pulse();
+send_step_pulse(1);
 dir = inb(FLOPPY_BASE + reg_dir);
 assert_bit_set(dir, DSKCHG);
 dir = inb(FLOPPY_BASE + reg_dir);
@@ -227,7 +223,14 @@ static void test_media_change(void)
 dir = inb(FLOPPY_BASE + reg_dir);
 assert_bit_set(dir, DSKCHG);
 
-send_step_pulse();
+send_step_pulse(0);
+dir = inb(FLOPPY_BASE + reg_dir);
+assert_bit_set(dir, DSKCHG);
+dir = inb(FLOPPY_BASE + reg_dir);
+assert_bit_set(dir, DSKCHG);
+
+/* Step to next track should clear DSKCHG bit. */
+send_step_pulse(1);
 dir = inb(FLOPPY_BASE + reg_dir);
 assert_bit_clear(dir, DSKCHG);
 dir = inb(FLOPPY_BASE + reg_dir);
@@ -243,7 +246,13 @@ static void test_media_change(void)
 dir = inb(FLOPPY_BASE + reg_dir);
 assert_bit_set(dir, DSKCHG);
 
-send_step_pulse();
+send_step_pulse(0);
+dir = inb(FLOPPY_BASE + reg_dir);
+assert_bit_set(dir, DSKCHG);
+dir = inb(FLOPPY_BASE + reg_dir);
+assert_bit_set(dir, DSKCHG);
+
+send_step_pulse(1);
 dir = inb(FLOPPY_BASE + reg_dir);
 assert_bit_set(dir, DSKCHG);
 dir = inb(FLOPPY_BASE + reg_dir);
-- 
1.7.6.5




Re: [Qemu-devel] [PATCH 23/25] fdc: Move floppy geometry guessing back from block.c

2012-07-09 Thread Anthony Liguori

On 07/09/2012 09:16 AM, Kevin Wolf wrote:

From: Markus Armbruster

Commit 5bbdbb46 moved it to block.c because "other geometry guessing
functions already reside in block.c".  Device-specific functionality
should be kept in device code, not the block layer.  Move it back.

Disk geometry guessing is still in block.c.  To be moved out in a
later patch series.

Bonus: the floppy type used in pc_cmos_init() now obviously matches
the one in the FDrive.  Before, we relied on
bdrv_get_floppy_geometry_hint() picking the same type both in
fd_revalidate() and in pc_cmos_init().

Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
  block.c  |  101 ---
  block.h  |   18 -
  hw/fdc.c |  122 -
  hw/fdc.h |   10 +-
  hw/pc.c  |   13 ++-
  5 files changed, 124 insertions(+), 140 deletions(-)

diff --git a/block.c b/block.c
index f2540b9..fa1789c 100644
--- a/block.c
+++ b/block.c
@@ -2272,107 +2272,6 @@ void bdrv_set_io_limits(BlockDriverState *bs,
  bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
  }

-/* Recognize floppy formats */
-typedef struct FDFormat {
-FDriveType drive;
-uint8_t last_sect;
-uint8_t max_track;
-uint8_t max_head;
-FDriveRate rate;
-} FDFormat;
-
-static const FDFormat fd_formats[] = {
-/* First entry is default format */
-/* 1.44 MB 3"1/2 floppy disks */
-{ FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
-/* 2.88 MB 3"1/2 floppy disks */
-{ FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
-/* 720 kB 3"1/2 floppy disks */
-{ FDRIVE_DRV_144,  9, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
-/* 1.2 MB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
-/* 720 kB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
-/* 360 kB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, },
-{ FDRIVE_DRV_120,  9, 40, 0, FDRIVE_RATE_300K, },
-{ FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
-{ FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
-/* 320 kB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120,  8, 40, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_120,  8, 40, 0, FDRIVE_RATE_250K, },
-/* 360 kB must match 5"1/4 better than 3"1/2... */
-{ FDRIVE_DRV_144,  9, 80, 0, FDRIVE_RATE_250K, },
-/* end */
-{ FDRIVE_DRV_NONE, -1, -1, 0, 0, },
-};
-
-void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
-   int *max_track, int *last_sect,
-   FDriveType drive_in, FDriveType *drive,
-   FDriveRate *rate)
-{
-const FDFormat *parse;
-uint64_t nb_sectors, size;
-int i, first_match, match;
-
-bdrv_get_geometry(bs,&nb_sectors);
-match = -1;
-first_match = -1;
-for (i = 0; ; i++) {
-parse =&fd_formats[i];
-if (parse->drive == FDRIVE_DRV_NONE) {
-break;
-}
-if (drive_in == parse->drive ||
-drive_in == FDRIVE_DRV_NONE) {
-size = (parse->max_head + 1) * parse->max_track *
-parse->last_sect;
-if (nb_sectors == size) {
-match = i;
-break;
-}
-if (first_match == -1) {
-first_match = i;
-}
-}
-}
-if (match == -1) {
-if (first_match == -1) {
-match = 1;
-} else {
-match = first_match;
-}
-parse =&fd_formats[match];
-}
-*nb_heads = parse->max_head + 1;
-*max_track = parse->max_track;
-*last_sect = parse->last_sect;
-*drive = parse->drive;
-*rate = parse->rate;
-}
-
  int bdrv_get_translation_hint(BlockDriverState *bs)
  {
   

[Qemu-devel] [PATCH 08/25] sheepdog: split outstanding list into inflight and pending

2012-07-09 Thread Kevin Wolf
From: MORITA Kazutaka 

outstanding_list_head is used for both pending and inflight requests.
This patch splits it and improves readability.

Signed-off-by: MORITA Kazutaka 
Signed-off-by: Kevin Wolf 
---
 block/sheepdog.c |   49 -
 1 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index d4e5e3a..f6cd517 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -259,7 +259,7 @@ typedef struct AIOReq {
 uint8_t flags;
 uint32_t id;
 
-QLIST_ENTRY(AIOReq) outstanding_aio_siblings;
+QLIST_ENTRY(AIOReq) aio_siblings;
 } AIOReq;
 
 enum AIOCBState {
@@ -305,7 +305,8 @@ typedef struct BDRVSheepdogState {
 Coroutine *co_recv;
 
 uint32_t aioreq_seq_num;
-QLIST_HEAD(outstanding_aio_head, AIOReq) outstanding_aio_head;
+QLIST_HEAD(inflight_aio_head, AIOReq) inflight_aio_head;
+QLIST_HEAD(pending_aio_head, AIOReq) pending_aio_head;
 } BDRVSheepdogState;
 
 static const char * sd_strerror(int err)
@@ -356,7 +357,7 @@ static const char * sd_strerror(int err)
  * Sheepdog I/O handling:
  *
  * 1. In sd_co_rw_vector, we send the I/O requests to the server and
- *link the requests to the outstanding_list in the
+ *link the requests to the inflight_list in the
  *BDRVSheepdogState.  The function exits without waiting for
  *receiving the response.
  *
@@ -384,9 +385,6 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, 
SheepdogAIOCB *acb,
 aio_req->flags = flags;
 aio_req->id = s->aioreq_seq_num++;
 
-QLIST_INSERT_HEAD(&s->outstanding_aio_head, aio_req,
-  outstanding_aio_siblings);
-
 acb->nr_pending++;
 return aio_req;
 }
@@ -395,7 +393,7 @@ static inline void free_aio_req(BDRVSheepdogState *s, 
AIOReq *aio_req)
 {
 SheepdogAIOCB *acb = aio_req->aiocb;
 
-QLIST_REMOVE(aio_req, outstanding_aio_siblings);
+QLIST_REMOVE(aio_req, aio_siblings);
 g_free(aio_req);
 
 acb->nr_pending--;
@@ -640,22 +638,21 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState 
*s, AIOReq *aio_req,
  * This function searchs pending requests to the object `oid', and
  * sends them.
  */
-static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid, 
uint32_t id)
+static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
 {
 AIOReq *aio_req, *next;
 SheepdogAIOCB *acb;
 int ret;
 
-QLIST_FOREACH_SAFE(aio_req, &s->outstanding_aio_head,
-   outstanding_aio_siblings, next) {
-if (id == aio_req->id) {
-continue;
-}
+QLIST_FOREACH_SAFE(aio_req, &s->pending_aio_head, aio_siblings, next) {
 if (aio_req->oid != oid) {
 continue;
 }
 
 acb = aio_req->aiocb;
+/* move aio_req from pending list to inflight one */
+QLIST_REMOVE(aio_req, aio_siblings);
+QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
 ret = add_aio_request(s, aio_req, acb->qiov->iov,
   acb->qiov->niov, 0, acb->aiocb_type);
 if (ret < 0) {
@@ -684,7 +681,7 @@ static void coroutine_fn aio_read_response(void *opaque)
 SheepdogAIOCB *acb;
 unsigned long idx;
 
-if (QLIST_EMPTY(&s->outstanding_aio_head)) {
+if (QLIST_EMPTY(&s->inflight_aio_head)) {
 goto out;
 }
 
@@ -695,8 +692,8 @@ static void coroutine_fn aio_read_response(void *opaque)
 goto out;
 }
 
-/* find the right aio_req from the outstanding_aio list */
-QLIST_FOREACH(aio_req, &s->outstanding_aio_head, outstanding_aio_siblings) 
{
+/* find the right aio_req from the inflight aio list */
+QLIST_FOREACH(aio_req, &s->inflight_aio_head, aio_siblings) {
 if (aio_req->id == rsp.id) {
 break;
 }
@@ -734,7 +731,7 @@ static void coroutine_fn aio_read_response(void *opaque)
  * create requests are not allowed, so we search the
  * pending requests here.
  */
-send_pending_req(s, vid_to_data_oid(s->inode.vdi_id, idx), rsp.id);
+send_pending_req(s, vid_to_data_oid(s->inode.vdi_id, idx));
 }
 break;
 case AIOCB_READ_UDATA:
@@ -786,7 +783,8 @@ static int aio_flush_request(void *opaque)
 {
 BDRVSheepdogState *s = opaque;
 
-return !QLIST_EMPTY(&s->outstanding_aio_head);
+return !QLIST_EMPTY(&s->inflight_aio_head) ||
+!QLIST_EMPTY(&s->pending_aio_head);
 }
 
 static int set_nodelay(int fd)
@@ -1103,7 +1101,8 @@ static int sd_open(BlockDriverState *bs, const char 
*filename, int flags)
 
 strstart(filename, "sheepdog:", (const char **)&filename);
 
-QLIST_INIT(&s->outstanding_aio_head);
+QLIST_INIT(&s->inflight_aio_head);
+QLIST_INIT(&s->pending_aio_head);
 s->fd = -1;
 
 memset(vdi, 0, sizeof(vdi));
@@ -1465,6 +1464,7 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
 iov.iov_len = sizeof

[Qemu-devel] [PATCH 07/25] sheepdog: make sure we don't free aiocb before sending all requests

2012-07-09 Thread Kevin Wolf
From: MORITA Kazutaka 

This patch increments the pending counter before sending requests, and
make sures that aiocb is not freed while sending them.

Signed-off-by: MORITA Kazutaka 
Signed-off-by: Kevin Wolf 
---
 block/sheepdog.c |   29 -
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 5dc1d7a..d4e5e3a 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -260,7 +260,6 @@ typedef struct AIOReq {
 uint32_t id;
 
 QLIST_ENTRY(AIOReq) outstanding_aio_siblings;
-QLIST_ENTRY(AIOReq) aioreq_siblings;
 } AIOReq;
 
 enum AIOCBState {
@@ -283,8 +282,7 @@ struct SheepdogAIOCB {
 void (*aio_done_func)(SheepdogAIOCB *);
 
 int canceled;
-
-QLIST_HEAD(aioreq_head, AIOReq) aioreq_head;
+int nr_pending;
 };
 
 typedef struct BDRVSheepdogState {
@@ -388,19 +386,19 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, 
SheepdogAIOCB *acb,
 
 QLIST_INSERT_HEAD(&s->outstanding_aio_head, aio_req,
   outstanding_aio_siblings);
-QLIST_INSERT_HEAD(&acb->aioreq_head, aio_req, aioreq_siblings);
 
+acb->nr_pending++;
 return aio_req;
 }
 
-static inline int free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
+static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
 {
 SheepdogAIOCB *acb = aio_req->aiocb;
+
 QLIST_REMOVE(aio_req, outstanding_aio_siblings);
-QLIST_REMOVE(aio_req, aioreq_siblings);
 g_free(aio_req);
 
-return !QLIST_EMPTY(&acb->aioreq_head);
+acb->nr_pending--;
 }
 
 static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
@@ -446,7 +444,7 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, 
QEMUIOVector *qiov,
 acb->canceled = 0;
 acb->coroutine = qemu_coroutine_self();
 acb->ret = 0;
-QLIST_INIT(&acb->aioreq_head);
+acb->nr_pending = 0;
 return acb;
 }
 
@@ -663,7 +661,7 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState 
*s, uint64_t oid, ui
 if (ret < 0) {
 error_report("add_aio_request is failed");
 free_aio_req(s, aio_req);
-if (QLIST_EMPTY(&acb->aioreq_head)) {
+if (!acb->nr_pending) {
 sd_finish_aiocb(acb);
 }
 }
@@ -684,7 +682,6 @@ static void coroutine_fn aio_read_response(void *opaque)
 int ret;
 AIOReq *aio_req = NULL;
 SheepdogAIOCB *acb;
-int rest;
 unsigned long idx;
 
 if (QLIST_EMPTY(&s->outstanding_aio_head)) {
@@ -755,8 +752,8 @@ static void coroutine_fn aio_read_response(void *opaque)
 error_report("%s", sd_strerror(rsp.result));
 }
 
-rest = free_aio_req(s, aio_req);
-if (!rest) {
+free_aio_req(s, aio_req);
+if (!acb->nr_pending) {
 /*
  * We've finished all requests which belong to the AIOCB, so
  * we can switch back to sd_co_readv/writev now.
@@ -1568,6 +1565,12 @@ static int coroutine_fn sd_co_rw_vector(void *p)
 }
 }
 
+/*
+ * Make sure we don't free the aiocb before we are done with all requests.
+ * This additional reference is dropped at the end of this function.
+ */
+acb->nr_pending++;
+
 while (done != total) {
 uint8_t flags = 0;
 uint64_t old_oid = 0;
@@ -1636,7 +1639,7 @@ static int coroutine_fn sd_co_rw_vector(void *p)
 done += len;
 }
 out:
-if (QLIST_EMPTY(&acb->aioreq_head)) {
+if (!--acb->nr_pending) {
 return acb->ret;
 }
 return 1;
-- 
1.7.6.5




[Qemu-devel] [PATCH 24/25] qtest: Tidy up temporary files properly

2012-07-09 Thread Kevin Wolf
From: Markus Armbruster 

Each test litters /tmp with several files: a pid file and two
sockets.  Tidy up.

Signed-off-by: Markus Armbruster 
Signed-off-by: Kevin Wolf 
---
 tests/libqtest.c |   29 -
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 071b6be..02d0392 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -40,6 +40,7 @@ struct QTestState
 bool irq_level[MAX_IRQ];
 GString *rx;
 gchar *pid_file;
+char *socket_path, *qmp_socket_path;
 };
 
 #define g_assert_no_errno(ret) do { \
@@ -88,8 +89,6 @@ QTestState *qtest_init(const char *extra_args)
 {
 QTestState *s;
 int sock, qmpsock, ret, i;
-gchar *socket_path;
-gchar *qmp_socket_path;
 gchar *pid_file;
 gchar *command;
 const char *qemu_binary;
@@ -98,14 +97,14 @@ QTestState *qtest_init(const char *extra_args)
 qemu_binary = getenv("QTEST_QEMU_BINARY");
 g_assert(qemu_binary != NULL);
 
-socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
-qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
-pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
-
 s = g_malloc(sizeof(*s));
 
-sock = init_socket(socket_path);
-qmpsock = init_socket(qmp_socket_path);
+s->socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
+s->qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
+pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
+
+sock = init_socket(s->socket_path);
+qmpsock = init_socket(s->qmp_socket_path);
 
 pid = fork();
 if (pid == 0) {
@@ -115,8 +114,8 @@ QTestState *qtest_init(const char *extra_args)
   "-qmp unix:%s,nowait "
   "-pidfile %s "
   "-machine accel=qtest "
-  "%s", qemu_binary, socket_path,
-  qmp_socket_path, pid_file,
+  "%s", qemu_binary, s->socket_path,
+  s->qmp_socket_path, pid_file,
   extra_args ?: "");
 
 ret = system(command);
@@ -133,9 +132,6 @@ QTestState *qtest_init(const char *extra_args)
 s->irq_level[i] = false;
 }
 
-g_free(socket_path);
-g_free(qmp_socket_path);
-
 /* Read the QMP greeting and then do the handshake */
 qtest_qmp(s, "");
 qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }");
@@ -160,6 +156,13 @@ void qtest_quit(QTestState *s)
 
 fclose(f);
 }
+
+unlink(s->pid_file);
+unlink(s->socket_path);
+unlink(s->qmp_socket_path);
+g_free(s->pid_file);
+g_free(s->socket_path);
+g_free(s->qmp_socket_path);
 }
 
 static void socket_sendf(int fd, const char *fmt, va_list ap)
-- 
1.7.6.5




[Qemu-devel] [PATCH v2] console: Implementing blinking of cursor

2012-07-09 Thread Jan Kiszka
Let the text console cursor blink at 2 HZ.

Signed-off-by: Jan Kiszka 
---

Changes in v2:
 - fixed semantic of CONSOLE_CURSOR_PERIOD and reduced frequency

I know there was a concern regarding the approach in general, but I
still consider it useful and visually more attractive than the static
cursor.

 console.c |   26 +-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/console.c b/console.c
index 6a463f5..7729c08 100644
--- a/console.c
+++ b/console.c
@@ -28,6 +28,7 @@
 //#define DEBUG_CONSOLE
 #define DEFAULT_BACKSCROLL 512
 #define MAX_CONSOLES 12
+#define CONSOLE_CURSOR_PERIOD 500
 
 #define QEMU_RGBA(r, g, b, a) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b))
 #define QEMU_RGB(r, g, b) QEMU_RGBA(r, g, b, 0xff)
@@ -139,6 +140,8 @@ struct TextConsole {
 TextCell *cells;
 int text_x[2], text_y[2], cursor_invalidate;
 int echo;
+int cursor_blink_state;
+QEMUTimer *cursor_timer;
 
 int update_x0;
 int update_y0;
@@ -615,7 +618,7 @@ static void console_show_cursor(TextConsole *s, int show)
 y += s->total_height;
 if (y < s->height) {
 c = &s->cells[y1 * s->width + x];
-if (show) {
+if (show && s->cursor_blink_state) {
 TextAttributes t_attrib = s->t_attrib_default;
 t_attrib.invers = !(t_attrib.invers); /* invert fg and bg */
 vga_putcharxy(s->ds, x, y, c->ch, &t_attrib);
@@ -1083,6 +1086,10 @@ void console_select(unsigned int index)
 s = consoles[index];
 if (s) {
 DisplayState *ds = s->ds;
+
+if (active_console->cursor_timer) {
+qemu_del_timer(active_console->cursor_timer);
+}
 active_console = s;
 if (ds_get_bits_per_pixel(s->ds)) {
 ds->surface = qemu_resize_displaysurface(ds, s->g_width, 
s->g_height);
@@ -1090,6 +1097,10 @@ void console_select(unsigned int index)
 s->ds->surface->width = s->width;
 s->ds->surface->height = s->height;
 }
+if (s->cursor_timer) {
+qemu_mod_timer(s->cursor_timer,
+   qemu_get_clock_ms(rt_clock) + CONSOLE_CURSOR_PERIOD / 2);
+}
 dpy_resize(s->ds);
 vga_hw_invalidate();
 }
@@ -1454,6 +1465,16 @@ static void text_console_set_echo(CharDriverState *chr, 
bool echo)
 s->echo = echo;
 }
 
+static void text_console_update_cursor(void *opaque)
+{
+TextConsole *s = opaque;
+
+s->cursor_blink_state ^= 1;
+vga_hw_invalidate();
+qemu_mod_timer(s->cursor_timer,
+   qemu_get_clock_ms(rt_clock) + CONSOLE_CURSOR_PERIOD / 2);
+}
+
 static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
 {
 TextConsole *s;
@@ -1482,6 +1503,9 @@ static void text_console_do_init(CharDriverState *chr, 
DisplayState *ds)
 s->g_height = ds_get_height(s->ds);
 }
 
+s->cursor_timer =
+qemu_new_timer_ms(rt_clock, text_console_update_cursor, s);
+
 s->hw_invalidate = text_console_invalidate;
 s->hw_text_update = text_console_update;
 s->hw = s;
-- 
1.7.3.4



Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

2012-07-09 Thread Kevin Wolf
Am 15.06.2012 22:47, schrieb Supriya Kannery:
> New command "block_set_hostcache" added for dynamically changing 
> host pagecache setting of a block device.
> 
> Usage: 
>  block_set_hostcache   
> = block device
> = on/off
> 
> Example:
>  (qemu) block_set_hostcache ide0-hd0 off
> 
> Signed-off-by: Supriya Kannery 
> 
> ---
>  block.c |   54 ++
>  block.h |2 ++
>  blockdev.c  |   26 ++
>  blockdev.h  |2 ++
>  hmp-commands.hx |   14 ++
>  qmp-commands.hx |   27 +++
>  6 files changed, 125 insertions(+)
> 
> Index: qemu/block.c
> ===
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,6 +858,35 @@ unlink_and_fail:
>  return ret;
>  }
>  
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> +BlockDriver *drv = bs->drv;
> +int ret = 0, open_flags;
> +
> +/* Quiesce IO for the given block device */
> +qemu_aio_flush();

bdrv_drain_all() should be used instead.

> +ret = bdrv_flush(bs);
> +if (ret != 0) {
> +qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
> +return ret;
> +}
> +open_flags = bs->open_flags;
> +bdrv_close(bs);
> +
> +ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> +if (ret < 0) {
> +/* Reopen failed. Try to open with original flags */
> +qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +ret = bdrv_open(bs, bs->filename, open_flags, drv);
> +if (ret < 0) {
> +/* Reopen failed with orig and modified flags */
> +abort();

Like Eric said, committing a broken version and then fixing it later in
the series isn't really nice. At least bs->drv = NULL; instead of
abort() is required. Starting with the real thing is probably even better.

Kevin



Re: [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures

2012-07-09 Thread Kevin Wolf
Am 15.06.2012 22:47, schrieb Supriya Kannery:
> New error classes defined for hostcache setting and data 
> sync error
> 
> Signed-off-by: Supriya Kannery 
> 
> ---
>  qerror.c |8 
>  qerror.h |6 ++
>  2 files changed, 14 insertions(+)
> 
> Index: qemu/qerror.c
> ===
> --- qemu.orig/qerror.c
> +++ qemu/qerror.c
> @@ -80,6 +80,10 @@ static const QErrorStringTable qerror_ta
>  .desc  = "The command %(name) has not been found",
>  },
>  {
> +.error_fmt = QERR_DATA_SYNC_FAILED,
> +.desc  = "Syncing of data failed for device '%(device)'",
> +},
> +{
>  .error_fmt = QERR_DEVICE_ENCRYPTED,
>  .desc  = "Device '%(device)' is encrypted",
>  },
> @@ -152,6 +156,10 @@ static const QErrorStringTable qerror_ta
>  .desc  = "The feature '%(name)' is not enabled",
>  },
>  {
> +.error_fmt = QERR_HOSTCACHE_NOT_CHANGED,
> +.desc  = "Could not change hostcache setting for '%(device)'",
> +},
> +{
>  .error_fmt = QERR_INVALID_BLOCK_FORMAT,
>  .desc  = "Invalid block format '%(name)'",
>  },
> Index: qemu/qerror.h
> ===
> --- qemu.orig/qerror.h
> +++ qemu/qerror.h
> @@ -82,6 +82,9 @@ QError *qobject_to_qerror(const QObject 
>  #define QERR_COMMAND_NOT_FOUND \
>  "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
>  
> +#define QERR_DATA_SYNC_FAILED \
> +"{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
> +
>  #define QERR_DEVICE_ENCRYPTED \
>  "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } 
> }"
>  
> @@ -136,6 +139,9 @@ QError *qobject_to_qerror(const QObject 
>  #define QERR_FEATURE_DISABLED \
>  "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
>  
> +#define QERR_HOSTCACHE_NOT_CHANGED \
> +"{ 'class': 'HostcacheNotChanged', 'data': { 'device': %s } }"
> +
>  #define QERR_INVALID_BLOCK_FORMAT \
>  "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"

In the light of the recent error handling discussion: Do we really need
two separate errors? Can we just reuse an existing one? Just
QERR_IO_ERROR could be good enough.

Kevin



[Qemu-devel] [PATCH 23/25] fdc: Move floppy geometry guessing back from block.c

2012-07-09 Thread Kevin Wolf
From: Markus Armbruster 

Commit 5bbdbb46 moved it to block.c because "other geometry guessing
functions already reside in block.c".  Device-specific functionality
should be kept in device code, not the block layer.  Move it back.

Disk geometry guessing is still in block.c.  To be moved out in a
later patch series.

Bonus: the floppy type used in pc_cmos_init() now obviously matches
the one in the FDrive.  Before, we relied on
bdrv_get_floppy_geometry_hint() picking the same type both in
fd_revalidate() and in pc_cmos_init().

Signed-off-by: Markus Armbruster 
Signed-off-by: Kevin Wolf 
---
 block.c  |  101 ---
 block.h  |   18 -
 hw/fdc.c |  122 -
 hw/fdc.h |   10 +-
 hw/pc.c  |   13 ++-
 5 files changed, 124 insertions(+), 140 deletions(-)

diff --git a/block.c b/block.c
index f2540b9..fa1789c 100644
--- a/block.c
+++ b/block.c
@@ -2272,107 +2272,6 @@ void bdrv_set_io_limits(BlockDriverState *bs,
 bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
 }
 
-/* Recognize floppy formats */
-typedef struct FDFormat {
-FDriveType drive;
-uint8_t last_sect;
-uint8_t max_track;
-uint8_t max_head;
-FDriveRate rate;
-} FDFormat;
-
-static const FDFormat fd_formats[] = {
-/* First entry is default format */
-/* 1.44 MB 3"1/2 floppy disks */
-{ FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
-/* 2.88 MB 3"1/2 floppy disks */
-{ FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
-/* 720 kB 3"1/2 floppy disks */
-{ FDRIVE_DRV_144,  9, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
-/* 1.2 MB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
-/* 720 kB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
-/* 360 kB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, },
-{ FDRIVE_DRV_120,  9, 40, 0, FDRIVE_RATE_300K, },
-{ FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
-{ FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
-/* 320 kB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120,  8, 40, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_120,  8, 40, 0, FDRIVE_RATE_250K, },
-/* 360 kB must match 5"1/4 better than 3"1/2... */
-{ FDRIVE_DRV_144,  9, 80, 0, FDRIVE_RATE_250K, },
-/* end */
-{ FDRIVE_DRV_NONE, -1, -1, 0, 0, },
-};
-
-void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
-   int *max_track, int *last_sect,
-   FDriveType drive_in, FDriveType *drive,
-   FDriveRate *rate)
-{
-const FDFormat *parse;
-uint64_t nb_sectors, size;
-int i, first_match, match;
-
-bdrv_get_geometry(bs, &nb_sectors);
-match = -1;
-first_match = -1;
-for (i = 0; ; i++) {
-parse = &fd_formats[i];
-if (parse->drive == FDRIVE_DRV_NONE) {
-break;
-}
-if (drive_in == parse->drive ||
-drive_in == FDRIVE_DRV_NONE) {
-size = (parse->max_head + 1) * parse->max_track *
-parse->last_sect;
-if (nb_sectors == size) {
-match = i;
-break;
-}
-if (first_match == -1) {
-first_match = i;
-}
-}
-}
-if (match == -1) {
-if (first_match == -1) {
-match = 1;
-} else {
-match = first_match;
-}
-parse = &fd_formats[match];
-}
-*nb_heads = parse->max_head + 1;
-*max_track = parse->max_track;
-*last_sect = parse->last_sect;
-*drive = parse->drive;
-*rate = parse->rate;
-}
-
 int bdrv_get_translation_hint(BlockDriverState *bs)
 {
 return bs->translation;
diff --git a/block.h 

Re: [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting

2012-07-09 Thread Kevin Wolf
Am 15.06.2012 23:07, schrieb Eric Blake:
> On 06/15/2012 02:47 PM, Supriya Kannery wrote:
>> Enhance "info block" to display hostcache setting for each
>> block device.
>>
> 
>>  ##
>>  { 'type': 'BlockInfo',
>>'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
>> -   'locked': 'bool', '*inserted': 'BlockDeviceInfo',
>> +   'locked': 'bool','hostcache': 'bool', '*inserted': 
>> 'BlockDeviceInfo',
> 
> space after comma
> 
> Since 'hostcache' was not present when talking to older qemu, should we
> mark it optional?

What does "optional" really mean? I always understood that it means that
whether the field exists or not depends on some runtime condition, not
on the qemu version. I would specify something like this, that always
exists in new qemu versions, in the "Since" section. Or maybe a separate
"Since" specification like in SpiceInfo for mouse-mode.

Kevin



[Qemu-devel] [PATCH 17/25] block: introduce bdrv_swap, implement bdrv_append on top of it

2012-07-09 Thread Kevin Wolf
From: Paolo Bonzini 

The new function can be made a bit nicer than bdrv_append.  It swaps the
whole contents, and then swaps back (using the usual t=a;a=b;b=t idiom)
the fields that need to stay on top.  Thus, it does not need explicit
bdrv_detach_dev, bdrv_iostatus_disable, etc.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block.c |  184 ++-
 block.h |1 +
 2 files changed, 100 insertions(+), 85 deletions(-)

diff --git a/block.c b/block.c
index 702821d..29857db 100644
--- a/block.c
+++ b/block.c
@@ -971,116 +971,130 @@ static void bdrv_rebind(BlockDriverState *bs)
 }
 }
 
-/*
- * Add new bs contents at the top of an image chain while the chain is
- * live, while keeping required fields on the top layer.
- *
- * This will modify the BlockDriverState fields, and swap contents
- * between bs_new and bs_top. Both bs_new and bs_top are modified.
- *
- * bs_new is required to be anonymous.
- *
- * This function does not create any image files.
- */
-void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
+static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
+ BlockDriverState *bs_src)
 {
-BlockDriverState tmp;
-
-/* bs_new must be anonymous */
-assert(bs_new->device_name[0] == '\0');
-
-tmp = *bs_new;
-
-/* there are some fields that need to stay on the top layer: */
-tmp.open_flags= bs_top->open_flags;
+/* move some fields that need to stay attached to the device */
+bs_dest->open_flags = bs_src->open_flags;
 
 /* dev info */
-tmp.dev_ops   = bs_top->dev_ops;
-tmp.dev_opaque= bs_top->dev_opaque;
-tmp.dev   = bs_top->dev;
-tmp.buffer_alignment  = bs_top->buffer_alignment;
-tmp.copy_on_read  = bs_top->copy_on_read;
+bs_dest->dev_ops= bs_src->dev_ops;
+bs_dest->dev_opaque = bs_src->dev_opaque;
+bs_dest->dev= bs_src->dev;
+bs_dest->buffer_alignment   = bs_src->buffer_alignment;
+bs_dest->copy_on_read   = bs_src->copy_on_read;
 
-tmp.enable_write_cache = bs_top->enable_write_cache;
+bs_dest->enable_write_cache = bs_src->enable_write_cache;
 
 /* i/o timing parameters */
-tmp.slice_time= bs_top->slice_time;
-tmp.slice_start   = bs_top->slice_start;
-tmp.slice_end = bs_top->slice_end;
-tmp.io_limits = bs_top->io_limits;
-tmp.io_base   = bs_top->io_base;
-tmp.throttled_reqs= bs_top->throttled_reqs;
-tmp.block_timer   = bs_top->block_timer;
-tmp.io_limits_enabled = bs_top->io_limits_enabled;
+bs_dest->slice_time = bs_src->slice_time;
+bs_dest->slice_start= bs_src->slice_start;
+bs_dest->slice_end  = bs_src->slice_end;
+bs_dest->io_limits  = bs_src->io_limits;
+bs_dest->io_base= bs_src->io_base;
+bs_dest->throttled_reqs = bs_src->throttled_reqs;
+bs_dest->block_timer= bs_src->block_timer;
+bs_dest->io_limits_enabled  = bs_src->io_limits_enabled;
 
 /* geometry */
-tmp.cyls  = bs_top->cyls;
-tmp.heads = bs_top->heads;
-tmp.secs  = bs_top->secs;
-tmp.translation   = bs_top->translation;
+bs_dest->cyls   = bs_src->cyls;
+bs_dest->heads  = bs_src->heads;
+bs_dest->secs   = bs_src->secs;
+bs_dest->translation= bs_src->translation;
 
 /* r/w error */
-tmp.on_read_error = bs_top->on_read_error;
-tmp.on_write_error= bs_top->on_write_error;
+bs_dest->on_read_error  = bs_src->on_read_error;
+bs_dest->on_write_error = bs_src->on_write_error;
 
 /* i/o status */
-tmp.iostatus_enabled  = bs_top->iostatus_enabled;
-tmp.iostatus  = bs_top->iostatus;
+bs_dest->iostatus_enabled   = bs_src->iostatus_enabled;
+bs_dest->iostatus   = bs_src->iostatus;
 
 /* dirty bitmap */
-tmp.dirty_count   = bs_top->dirty_count;
-tmp.dirty_bitmap  = bs_top->dirty_bitmap;
-assert(bs_new->dirty_bitmap == NULL);
+bs_dest->dirty_count= bs_src->dirty_count;
+bs_dest->dirty_bitmap   = bs_src->dirty_bitmap;
 
 /* job */
-tmp.in_use= bs_top->in_use;
-tmp.job   = bs_top->job;
-assert(bs_new->job == NULL);
+bs_dest->in_use = bs_src->in_use;
+bs_dest->job= bs_src->job;
 
 /* keep the same entry in bdrv_states */
-pstrcpy(tmp.device_name, sizeof(tmp.device_name), bs_top->device_name);
-tmp.list = bs_top->list;
+pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name),
+bs_src->device_name);
+bs_dest->list = bs_src->list;
+}
 
-/* The contents of 'tmp' will become bs_top, as we are
- * swapping bs_new and bs_top contents. */
-tmp.backing_h

[Qemu-devel] [PATCH 3/3] apic: Defer interrupt updates to VCPU thread

2012-07-09 Thread Jan Kiszka
KVM performs TPR raising asynchronously to QEMU, specifically outside
QEMU's global lock. When an interrupt is injected into the APIC and TPR
is checked to decide if this can be delivered, a stale TPR value may be
used, causing spurious interrupts in the end.

Fix this by deferring apic_update_irq to the context of the target VCPU.
We introduce a new interrupt flag for this, CPU_INTERRUPT_POLL. When it
is set, the VCPU calls apic_poll_irq before checking for further pending
interrupts. To avoid special-casing KVM, we also implement this logic
for TCG mode.

Signed-off-by: Jan Kiszka 
---
 cpu-exec.c |6 ++
 hw/apic.c  |5 -
 hw/apic.h  |1 +
 hw/apic_internal.h |1 -
 target-i386/cpu.h  |4 +++-
 target-i386/kvm.c  |4 
 6 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 08c35f7..fc185a4 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -288,6 +288,12 @@ int cpu_exec(CPUArchState *env)
 }
 #endif
 #if defined(TARGET_I386)
+#if !defined(CONFIG_USER_ONLY)
+if (interrupt_request & CPU_INTERRUPT_POLL) {
+env->interrupt_request &= ~CPU_INTERRUPT_POLL;
+apic_poll_irq(env->apic_state);
+}
+#endif
 if (interrupt_request & CPU_INTERRUPT_INIT) {
 cpu_svm_check_intercept_param(env, SVM_EXIT_INIT,
   0);
diff --git a/hw/apic.c b/hw/apic.c
index 5b8f3e8..38e 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -16,6 +16,7 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see 
  */
+#include "qemu-thread.h"
 #include "apic_internal.h"
 #include "apic.h"
 #include "ioapic.h"
@@ -361,7 +362,9 @@ static void apic_update_irq(APICCommonState *s)
 if (!(s->spurious_vec & APIC_SV_ENABLE)) {
 return;
 }
-if (apic_irq_pending(s) > 0) {
+if (!qemu_cpu_is_self(s->cpu_env)) {
+cpu_interrupt(s->cpu_env, CPU_INTERRUPT_POLL);
+} else if (apic_irq_pending(s) > 0) {
 cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
 }
 }
diff --git a/hw/apic.h b/hw/apic.h
index 62179ce..a89542b 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -20,6 +20,7 @@ void apic_init_reset(DeviceState *s);
 void apic_sipi(DeviceState *s);
 void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
TPRAccess access);
+void apic_poll_irq(DeviceState *d);
 
 /* pc.c */
 int cpu_is_bsp(CPUX86State *env);
diff --git a/hw/apic_internal.h b/hw/apic_internal.h
index 60a6a8b..4d8ff49 100644
--- a/hw/apic_internal.h
+++ b/hw/apic_internal.h
@@ -141,7 +141,6 @@ void apic_report_irq_delivered(int delivered);
 bool apic_next_timer(APICCommonState *s, int64_t current_time);
 void apic_enable_tpr_access_reporting(DeviceState *d, bool enable);
 void apic_enable_vapic(DeviceState *d, target_phys_addr_t paddr);
-void apic_poll_irq(DeviceState *d);
 
 void vapic_report_tpr_access(DeviceState *dev, void *cpu, target_ulong ip,
  TPRAccess access);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f257c97..1f6f14f 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -477,6 +477,7 @@
  for syscall instruction */
 
 /* i386-specific interrupt pending bits.  */
+#define CPU_INTERRUPT_POLL  CPU_INTERRUPT_TGT_EXT_1
 #define CPU_INTERRUPT_SMI   CPU_INTERRUPT_TGT_EXT_2
 #define CPU_INTERRUPT_NMI   CPU_INTERRUPT_TGT_EXT_3
 #define CPU_INTERRUPT_MCE   CPU_INTERRUPT_TGT_EXT_4
@@ -1047,7 +1048,8 @@ static inline void cpu_clone_regs(CPUX86State *env, 
target_ulong newsp)
 
 static inline bool cpu_has_work(CPUX86State *env)
 {
-return ((env->interrupt_request & CPU_INTERRUPT_HARD) &&
+return ((env->interrupt_request & (CPU_INTERRUPT_HARD |
+   CPU_INTERRUPT_POLL)) &&
 (env->eflags & IF_MASK)) ||
(env->interrupt_request & (CPU_INTERRUPT_NMI |
   CPU_INTERRUPT_INIT |
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0d0d8f6..cfe60bc 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1727,6 +1727,10 @@ int kvm_arch_process_async_events(CPUX86State *env)
 return 0;
 }
 
+if (env->interrupt_request & CPU_INTERRUPT_POLL) {
+env->interrupt_request &= ~CPU_INTERRUPT_POLL;
+apic_poll_irq(env->apic_state);
+}
 if (((env->interrupt_request & CPU_INTERRUPT_HARD) &&
  (env->eflags & IF_MASK)) ||
 (env->interrupt_request & CPU_INTERRUPT_NMI)) {
-- 
1.7.3.4




[Qemu-devel] [PATCH 2/3] apic: Reevaluate pending interrupts on LVT_LINT0 changes

2012-07-09 Thread Jan Kiszka
When the guest modifies the LVT_LINT0 register, we need to check if some
pending PIC interrupt can now be delivered.

Signed-off-by: Jan Kiszka 
---
 hw/apic.c |   18 ++
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index e65a35f..5b8f3e8 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -532,6 +532,15 @@ static void apic_deliver(DeviceState *d, uint8_t dest, 
uint8_t dest_mode,
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
 }
 
+static bool apic_check_pic(APICCommonState *s)
+{
+if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
+return false;
+}
+apic_deliver_pic_intr(&s->busdev.qdev, 1);
+return true;
+}
+
 int apic_get_interrupt(DeviceState *d)
 {
 APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
@@ -559,9 +568,7 @@ int apic_get_interrupt(DeviceState *d)
 apic_sync_vapic(s, SYNC_TO_VAPIC);
 
 /* re-inject if there is still a pending PIC interrupt */
-if (apic_accept_pic_intr(&s->busdev.qdev) && pic_get_output(isa_pic)) {
-apic_deliver_pic_intr(&s->busdev.qdev, 1);
-}
+apic_check_pic(s);
 
 apic_update_irq(s);
 
@@ -804,8 +811,11 @@ static void apic_mem_writel(void *opaque, 
target_phys_addr_t addr, uint32_t val)
 {
 int n = index - 0x32;
 s->lvt[n] = val;
-if (n == APIC_LVT_TIMER)
+if (n == APIC_LVT_TIMER) {
 apic_timer_update(s, qemu_get_clock_ns(vm_clock));
+} else if (n == APIC_LVT_LINT0 && apic_check_pic(s)) {
+apic_update_irq(s);
+}
 }
 break;
 case 0x38:
-- 
1.7.3.4




[Qemu-devel] [PATCH 1/3] apic: Resolve potential endless loop around apic_update_irq

2012-07-09 Thread Jan Kiszka
Commit d96e173769 refactored the reinjection of pending PIC interrupts.
However, it missed the potential loop of apic_update_irq ->
apic_deliver_pic_intr -> apic_local_deliver -> apic_set_irq ->
apic_update_irq that /could/ occur if LINT0 is injected as APIC_DM_FIXED
and that vector is currently blocked via TPR.

Resolve this by reinjecting only where it matters: inside
apic_get_interrupt. This function may clear a vector while a
PIC-originated reason still exists.

Signed-off-by: Jan Kiszka 
---
 hw/apic.c |   10 +++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 60552df..e65a35f 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -363,9 +363,6 @@ static void apic_update_irq(APICCommonState *s)
 }
 if (apic_irq_pending(s) > 0) {
 cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
-} else if (apic_accept_pic_intr(&s->busdev.qdev) &&
-   pic_get_output(isa_pic)) {
-apic_deliver_pic_intr(&s->busdev.qdev, 1);
 }
 }
 
@@ -560,7 +557,14 @@ int apic_get_interrupt(DeviceState *d)
 reset_bit(s->irr, intno);
 set_bit(s->isr, intno);
 apic_sync_vapic(s, SYNC_TO_VAPIC);
+
+/* re-inject if there is still a pending PIC interrupt */
+if (apic_accept_pic_intr(&s->busdev.qdev) && pic_get_output(isa_pic)) {
+apic_deliver_pic_intr(&s->busdev.qdev, 1);
+}
+
 apic_update_irq(s);
+
 return intno;
 }
 
-- 
1.7.3.4




[Qemu-devel] [PATCH 0/3] apic: Fixes for userspace model

2012-07-09 Thread Jan Kiszka
As Avi noted recently, there is a problem in way we inject interrupts
into the userspace APIC under KVM: The TRP check over the iothread may
race with the VCPU raising the TPR value while in KVM mode. Patch 3
addresses this issue.

The other two patches fix problems I came across while thinking about
the first one.

Who would like to process this series, up/master? Or should it go in
directly?

Jan Kiszka (3):
  apic: Resolve potential endless loop around apic_update_irq
  apic: Reevaluate pending interrupts on LVT_LINT0 changes
  apic: Defer interrupt updates to VCPU thread

 cpu-exec.c |6 ++
 hw/apic.c  |   27 ++-
 hw/apic.h  |1 +
 hw/apic_internal.h |1 -
 target-i386/cpu.h  |4 +++-
 target-i386/kvm.c  |4 
 6 files changed, 36 insertions(+), 7 deletions(-)

-- 
1.7.3.4




[Qemu-devel] [PATCH 12/25] blkdebug: pass getlength to underlying file

2012-07-09 Thread Kevin Wolf
From: Paolo Bonzini 

This is required when using blkdebug with raw format.  Unlike qcow2/QED,
raw asks blkdebug for the length of the file, it doesn't get it from
a header.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block/blkdebug.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 1f79ef2..b084a23 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -429,6 +429,11 @@ static void blkdebug_debug_event(BlockDriverState *bs, 
BlkDebugEvent event)
 }
 }
 
+static int64_t blkdebug_getlength(BlockDriverState *bs)
+{
+return bdrv_getlength(bs->file);
+}
+
 static BlockDriver bdrv_blkdebug = {
 .format_name= "blkdebug",
 .protocol_name  = "blkdebug",
@@ -437,6 +442,7 @@ static BlockDriver bdrv_blkdebug = {
 
 .bdrv_file_open = blkdebug_open,
 .bdrv_close = blkdebug_close,
+.bdrv_getlength = blkdebug_getlength,
 
 .bdrv_aio_readv = blkdebug_aio_readv,
 .bdrv_aio_writev= blkdebug_aio_writev,
-- 
1.7.6.5




Re: [Qemu-devel] [PATCH] disas: Fix printing of addresses in disassembly

2012-07-09 Thread Andreas Färber
Am 09.07.2012 15:26, schrieb Peter Maydell:
> On 9 July 2012 14:19, Andreas Färber  wrote:
>> Am 25.06.2012 16:55, schrieb Peter Maydell:
>>> In our disassembly code, the bfd_vma type is always 64 bits,
>>> even if the target's virtual address width is only 32 bits. This
>>> means that when we print out addresses we need to truncate them
>>> to 32 bits, to avoid odd output which has incorrectly sign-extended
>>> a value to 64 bits, for instance this ARM example:
>>> 0x80479a60:  e59f4088 ldr  r4, [pc, #136]  ; 0x80479a4f
>>>
>>> (It would also be possible to truncate before passing the address
>>> to info->print_address_func(), but truncating in the final print
>>> function is the same approach that binutils takes to this problem.)
> 
>>> +/* Print address in hex, truncated to the width of a target virtual 
>>> address. */
>>> +static void
>>> +generic_print_target_address(bfd_vma addr, struct disassemble_info *info)
>>> +{
>>> +uint64_t mask = ~0ULL >> (64 - TARGET_VIRT_ADDR_SPACE_BITS);
>>> +generic_print_address(addr & mask, info);
>>> +}
> 
>> I wonder if TARGET_VIRT_ADDR_SPACE_BITS is the correct factor to use
>> here though? Might sizeof(target_phys_addr_t) * 8 be safer?
> 
> That will give the wrong answer for ARM (when my LPAE patchset lands):
> ARM virtual addresses are 32 bits but sizeof(target_phys_addr_t) * 8
> will be 64.

Sorry, I'm mixing things up...

  VAS / PAS
arm32 / 40
i386   32 / 36
x86_64 47 / 52
ppc32 / 36
ppc64  64 / 62

It's the physical address space where ppc64 is the oddball, so:

Reviewed-by: Andreas Färber 

>> I'm thinking
>> of the possibility of having an alias in the 64-bit address space point
>> into the actual 36/48/... virtual address space. I have a ppc64 ld
>> instruction in mind, for which a full 64-bit register would be set up
>> that could not fully be represented in the virtual address space.
> 
> It would be helpful to check how the ppc disassembler handles that
> ld insn. It may well either (a) not print the resulting address at
> all or (b) print it itself. However, if the resulting actual address
> is a virtual address space then the right thing to print would be
> the truncated version, I think, assuming that is what the hardware will
> actually do the load from.

I made a thinko: The disassembler would only show the register numbers
for the ld instruction, and the register would be loaded by up to five
instructions with 16-bit immediate (shifted) loads. So there would be no
problem with the operands.
The value printing at runtime is handled by the gdb stub instead.

Andreas

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





Re: [Qemu-devel] [PATCH] PPC: e500: reconditionalize on CONFIG_FDT

2012-07-09 Thread Alexander Graf

On 09.07.2012, at 16:36, Scott Wood wrote:

> On 07/09/2012 09:07 AM, Andreas Färber wrote:
>> Am 09.07.2012 16:04, schrieb Scott Wood:
>>> Recent patches "PPC: e500: rename mpc8544ds into generic file", "PPC:
>>> e500: split mpc8544ds machine from generic e500 code", and "PPC: e500:
>>> add generic e500 platform" moved certain e500-related files to the bottom
>>> of the makefile because they're now in ppc/, but the dependency on
>>> CONFIG_FDT was accidentally dropped.  This broke the build when FDT
>>> support is not enabled.
>>> 
>>> Signed-off-by: Scott Wood 
>> 
>> Reviewed-by: Andreas Färber 
>> 
>> But this can probably still be squashed into the original patches for
>> bisectability I hope?
> 
> OK, wasn't sure what the rebase policy was once the patch is in a
> maintainer tree.  Alex, do you want me to resend the original patchset
> with the fix squashed?

No worries, I'll just squash it in myself. I keep my trees rebaseable. That 
heavily improves bisectability and as long as nobody complains, I'd like to 
keep it that way :).


Alex




Re: [Qemu-devel] CVE-2011-2212: has it been actually fixed?

2012-07-09 Thread Anthony Liguori

On 07/07/2012 08:37 AM, Michael Tokarev wrote:

I come across a patch in ububtu qemu-kvm package, this:

From: Nelson Elhage
Date: Thu, 19 May 2011 13:23:17 -0400
Subject: [PATCH] virtqueue: Sanity-check the length of indirect descriptors.

We were previously allowing arbitrarily-long descriptors, which could lead to a
buffer overflow in the qemu-kvm process.


I don't have the original thread handy, but while the CVE was still embargoed, 
we made some changes to Nelson's original patch which is what led to Michael's 
patch.


We had a test case for the bug and confirmed that Michael's patch fixed that 
test case.


Regards,

Anthony Liguori



Index: qemu-kvm-1.1~rc+dfsg/hw/virtio.c
===
--- qemu-kvm-1.1~rc+dfsg.orig/hw/virtio.c   2012-06-01 01:19:22.0 
+
+++ qemu-kvm-1.1~rc+dfsg/hw/virtio.c2012-06-12 19:31:02.336250076 +
@@ -370,6 +370,11 @@
  max = vring_desc_len(desc_pa, i) / sizeof(VRingDesc);
  num_bufs = i = 0;
  desc_pa = vring_desc_addr(desc_pa, i);
+
+if (max>  VIRTQUEUE_MAX_SIZE) {
+error_report("Too-large indirect descriptor");
+exit(1);
+}
  }

  do {
@@ -443,6 +448,11 @@
  max = vring_desc_len(desc_pa, i) / sizeof(VRingDesc);
  desc_pa = vring_desc_addr(desc_pa, i);
  i = 0;
+
+if (max>  VIRTQUEUE_MAX_SIZE) {
+error_report("Too-large indirect descriptor");
+exit(1);
+}
  }

  /* Collect all the descriptors */


And I wonder if it is still needed.  The mentioned CVE-2011-2212
has been fixed before 0.15, by the following:


commit c8eac1cfa1e9104a658b4614ada758861b8d823a
Author: Michael S. Tsirkin
Date:   Mon Jun 20 13:42:27 2011 +0300

 virtio: fix indirect descriptor buffer overflow

 We were previously allowing arbitrarily-long indirect descriptors, which
 could lead to a buffer overflow in qemu-kvm process.

 CVE-2011-2212

 Signed-off-by: Michael S. Tsirkin

diff --git a/hw/virtio.c b/hw/virtio.c
index cc47a06..a8f4940 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -449,9 +449,17 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
  struct iovec *sg;

  if (vring_desc_flags(desc_pa, i)&  VRING_DESC_F_WRITE) {
+if (elem->in_num>= ARRAY_SIZE(elem->in_sg)) {
+error_report("Too many write descriptors in indirect table");
+exit(1);
+}
  elem->in_addr[elem->in_num] = vring_desc_addr(desc_pa, i);
  sg =&elem->in_sg[elem->in_num++];
  } else {
+if (elem->out_num>= ARRAY_SIZE(elem->out_sg)) {
+error_report("Too many read descriptors in indirect table");
+exit(1);
+}
  elem->out_addr[elem->out_num] = vring_desc_addr(desc_pa, i);
  sg =&elem->out_sg[elem->out_num++];
  }


But this one - apparently - fixes a different codepath, no?

Thanks,

/mjt






Re: [Qemu-devel] [RFC] introduce a dynamic library to expose qemu block API

2012-07-09 Thread Christoph Hellwig
On Mon, Jul 09, 2012 at 04:54:08PM +0800, Wenchao Xia wrote:
> Hi, Paolo and folks,
>   qemu have good capabilities to access different virtual disks, I want
> to expose its block layer API to let 3rd party program linked in, such
> as management stack or block tools, to access images data directly.
> 
> Following is the objects:
>   (1) API to write/read block device at offset.
>   (2) Determine the image type,qcow2/qed/raw
>   (3) Determine which blocks are allocated.
>   (4) Determine backing file.

Sounds like you want a procedural interface for that.  At least for (1)
I have patches I'll submit soon to add qemu img read/write commands.




[Qemu-devel] [PATCH 15/25] raw: hook into blkdebug

2012-07-09 Thread Kevin Wolf
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block/raw.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/block/raw.c b/block/raw.c
index 09d9b48..ff34ea4 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -12,12 +12,14 @@ static int raw_open(BlockDriverState *bs, int flags)
 static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
  int nb_sectors, QEMUIOVector *qiov)
 {
+BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
 return bdrv_co_readv(bs->file, sector_num, nb_sectors, qiov);
 }
 
 static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
   int nb_sectors, QEMUIOVector *qiov)
 {
+BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
 return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
 }
 
-- 
1.7.6.5




[Qemu-devel] [PATCH v3] sheepdog: do not blindly memset all read buffers

2012-07-09 Thread Christoph Hellwig
Only buffers that map to unallocated blocks need to be zeroed.

Signed-off-by: Christoph Hellwig 

---
 block/sheepdog.c |   37 ++---
 1 file changed, 18 insertions(+), 19 deletions(-)

Index: qemu/block/sheepdog.c
===
--- qemu.orig/block/sheepdog.c  2012-07-09 16:16:01.216467182 +0200
+++ qemu/block/sheepdog.c   2012-07-09 16:22:11.039798306 +0200
@@ -1556,18 +1556,25 @@ static int coroutine_fn sd_co_rw_vector(
 
 len = MIN(total - done, SD_DATA_OBJ_SIZE - offset);
 
-if (!inode->data_vdi_id[idx]) {
-if (acb->aiocb_type == AIOCB_READ_UDATA) {
+switch (acb->aiocb_type) {
+case AIOCB_READ_UDATA:
+if (!inode->data_vdi_id[idx]) {
+qemu_iovec_memset_skip(acb->qiov, 0, len, done);
 goto done;
 }
-
-create = 1;
-} else if (acb->aiocb_type == AIOCB_WRITE_UDATA
-   && !is_data_obj_writable(inode, idx)) {
-/* Copy-On-Write */
-create = 1;
-old_oid = oid;
-flags = SD_FLAG_CMD_COW;
+break;
+case AIOCB_WRITE_UDATA:
+if (!inode->data_vdi_id[idx]) {
+create = 1;
+} else if (!is_data_obj_writable(inode, idx)) {
+/* Copy-On-Write */
+create = 1;
+old_oid = oid;
+flags = SD_FLAG_CMD_COW;
+}
+break;
+default:
+break;
 }
 
 if (create) {
@@ -1655,20 +1662,12 @@ static coroutine_fn int sd_co_readv(Bloc
int nb_sectors, QEMUIOVector *qiov)
 {
 SheepdogAIOCB *acb;
-int i, ret;
+int ret;
 
 acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors, NULL, NULL);
 acb->aiocb_type = AIOCB_READ_UDATA;
 acb->aio_done_func = sd_finish_aiocb;
 
-/*
- * TODO: we can do better; we don't need to initialize
- * blindly.
- */
-for (i = 0; i < qiov->niov; i++) {
-memset(qiov->iov[i].iov_base, 0, qiov->iov[i].iov_len);
-}
-
 ret = sd_co_rw_vector(acb);
 if (ret <= 0) {
 qemu_aio_release(acb);



[Qemu-devel] [PATCH 19/25] fdc: fix interrupt handling

2012-07-09 Thread Kevin Wolf
From: Pavel Hrdina 

If you call the SENSE INTERRUPT STATUS command while there is no interrupt
waiting you get as result unknown command.

Fixed status0 register handling for read/write/format commands.

Signed-off-by: Pavel Hrdina 
Signed-off-by: Kevin Wolf 
---
 hw/fdc.c |   34 +-
 1 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index 0270264..e28841c 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -307,6 +307,9 @@ enum {
 };
 
 enum {
+FD_SR0_DS0  = 0x01,
+FD_SR0_DS1  = 0x02,
+FD_SR0_HEAD = 0x04,
 FD_SR0_EQPMT= 0x10,
 FD_SR0_SEEK = 0x20,
 FD_SR0_ABNTERM  = 0x40,
@@ -972,14 +975,15 @@ static void fdctrl_reset_fifo(FDCtrl *fdctrl)
 }
 
 /* Set FIFO status for the host to read */
-static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len, int do_irq)
+static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len, uint8_t status0)
 {
 fdctrl->data_dir = FD_DIR_READ;
 fdctrl->data_len = fifo_len;
 fdctrl->data_pos = 0;
 fdctrl->msr |= FD_MSR_CMDBUSY | FD_MSR_RQM | FD_MSR_DIO;
-if (do_irq)
-fdctrl_raise_irq(fdctrl, 0x00);
+if (status0) {
+fdctrl_raise_irq(fdctrl, status0);
+}
 }
 
 /* Set an error: unimplemented/unknown command */
@@ -1044,10 +1048,12 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, 
uint8_t status0,
 FDrive *cur_drv;
 
 cur_drv = get_cur_drv(fdctrl);
+fdctrl->status0 = status0 | FD_SR0_SEEK | (cur_drv->head << 2) |
+  GET_CUR_DRV(fdctrl);
+
 FLOPPY_DPRINTF("transfer status: %02x %02x %02x (%02x)\n",
-   status0, status1, status2,
-   status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl));
-fdctrl->fifo[0] = status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
+   status0, status1, status2, fdctrl->status0);
+fdctrl->fifo[0] = fdctrl->status0;
 fdctrl->fifo[1] = status1;
 fdctrl->fifo[2] = status2;
 fdctrl->fifo[3] = cur_drv->track;
@@ -1060,7 +1066,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t 
status0,
 }
 fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO;
 fdctrl->msr &= ~FD_MSR_NONDMA;
-fdctrl_set_fifo(fdctrl, 7, 1);
+fdctrl_set_fifo(fdctrl, 7, fdctrl->status0);
 }
 
 /* Prepare a data transfer (either DMA or FIFO) */
@@ -1175,7 +1181,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int 
direction)
 if (direction != FD_DIR_WRITE)
 fdctrl->msr |= FD_MSR_DIO;
 /* IO based transfer: calculate len */
-fdctrl_raise_irq(fdctrl, 0x00);
+fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
 
 return;
 }
@@ -1604,16 +1610,18 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl 
*fdctrl, int direction)
 {
 FDrive *cur_drv = get_cur_drv(fdctrl);
 
-if(fdctrl->reset_sensei > 0) {
+if (fdctrl->reset_sensei > 0) {
 fdctrl->fifo[0] =
 FD_SR0_RDYCHG + FD_RESET_SENSEI_COUNT - fdctrl->reset_sensei;
 fdctrl->reset_sensei--;
+} else if (!(fdctrl->sra & FD_SRA_INTPEND)) {
+fdctrl->fifo[0] = FD_SR0_INVCMD;
+fdctrl_set_fifo(fdctrl, 1, 0);
+return;
 } else {
-/* XXX: status0 handling is broken for read/write
-   commands, so we do this hack. It should be suppressed
-   ASAP */
 fdctrl->fifo[0] =
-FD_SR0_SEEK | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
+(fdctrl->status0 & ~(FD_SR0_HEAD | FD_SR0_DS1 | FD_SR0_DS0))
+| GET_CUR_DRV(fdctrl);
 }
 
 fdctrl->fifo[1] = cur_drv->track;
-- 
1.7.6.5




[Qemu-devel] [PATCH] vl.c: Don't print errno after failed qemu_chr_new()

2012-07-09 Thread Peter Maydell
The qemu_chr_new() function doesn't set errno on failure, so
don't print strerror(errno) on the error handling path when
dealing with the -serial, -parallel and -virtioconsole arguments.
This avoids nonsensical error messages like:
  $ ./arm-softmmu/qemu-system-arm -serial wombat
  qemu: could not open serial device 'wombat': Success

We also rephrase the message slightly to make it a little clearer
that we're expecting the name of a QEMU chr backend rather than
a host or guest serial/parallel/etc device.

Reported-by: Christian Müller 
Signed-off-by: Peter Maydell 
---
 vl.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/vl.c b/vl.c
index 1329c30..8d49ff0 100644
--- a/vl.c
+++ b/vl.c
@@ -1985,8 +1985,8 @@ static int serial_parse(const char *devname)
 snprintf(label, sizeof(label), "serial%d", index);
 serial_hds[index] = qemu_chr_new(label, devname, NULL);
 if (!serial_hds[index]) {
-fprintf(stderr, "qemu: could not open serial device '%s': %s\n",
-devname, strerror(errno));
+fprintf(stderr, "qemu: could not connect serial device"
+" to character backend '%s'\n", devname);
 return -1;
 }
 index++;
@@ -2007,8 +2007,8 @@ static int parallel_parse(const char *devname)
 snprintf(label, sizeof(label), "parallel%d", index);
 parallel_hds[index] = qemu_chr_new(label, devname, NULL);
 if (!parallel_hds[index]) {
-fprintf(stderr, "qemu: could not open parallel device '%s': %s\n",
-devname, strerror(errno));
+fprintf(stderr, "qemu: could not connect parallel device"
+" to character backend '%s'\n", devname);
 return -1;
 }
 index++;
@@ -2042,8 +2042,8 @@ static int virtcon_parse(const char *devname)
 snprintf(label, sizeof(label), "virtcon%d", index);
 virtcon_hds[index] = qemu_chr_new(label, devname, NULL);
 if (!virtcon_hds[index]) {
-fprintf(stderr, "qemu: could not open virtio console '%s': %s\n",
-devname, strerror(errno));
+fprintf(stderr, "qemu: could not connect virtio console"
+" to character backend '%s'\n", devname);
 return -1;
 }
 qemu_opt_set(dev_opts, "chardev", label);
-- 
1.7.1




[Qemu-devel] [PATCH] hw/pl011.c: Avoid crash on read when no chr backend present

2012-07-09 Thread Peter Maydell
Add a missing guard that meant we would segfault if the guest read
UARTDR on a PL011 serial device which had no chr backend connected.
(This didn't happen for Linux guests because Linux reads the flags
register and doesn't try to read the UART if it's empty.)

Reported-by: Christian Müller 
Signed-off-by: Peter Maydell 
---
 hw/pl011.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/pl011.c b/hw/pl011.c
index 8a5a8f5..3245702 100644
--- a/hw/pl011.c
+++ b/hw/pl011.c
@@ -78,7 +78,9 @@ static uint64_t pl011_read(void *opaque, target_phys_addr_t 
offset,
 if (s->read_count == s->read_trigger - 1)
 s->int_level &= ~ PL011_INT_RX;
 pl011_update(s);
-qemu_chr_accept_input(s->chr);
+if (s->chr) {
+qemu_chr_accept_input(s->chr);
+}
 return c;
 case 1: /* UARTCR */
 return 0;
-- 
1.7.1




Re: [Qemu-devel] [PATCH] sheepdog: always use coroutine-based network functions

2012-07-09 Thread Kevin Wolf
Am 04.07.2012 18:41, schrieb MORITA Kazutaka:
> This reduces some code duplication.
> 
> Signed-off-by: MORITA Kazutaka 

Thanks, applied to the block branch.

Kevin



[Qemu-devel] [PATCH 18/25] fdc: rewrite seek and DSKCHG bit handling

2012-07-09 Thread Kevin Wolf
From: Pavel Hrdina 

This bit is cleared on every successful seek to a different track (cylinder).
The seek is also called on revalidate or on read/write/format commands which
also clear the DSKCHG bit.

Signed-off-by: Pavel Hrdina 
Signed-off-by: Kevin Wolf 
---
 hw/fdc.c |   79 -
 1 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index 5b3224b..0270264 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -153,8 +153,12 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t 
track, uint8_t sect,
 }
 #endif
 drv->head = head;
-if (drv->track != track)
+if (drv->track != track) {
+if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
+drv->media_changed = 0;
+}
 ret = 1;
+}
 drv->track = track;
 drv->sect = sect;
 }
@@ -170,9 +174,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t 
track, uint8_t sect,
 static void fd_recalibrate(FDrive *drv)
 {
 FLOPPY_DPRINTF("recalibrate\n");
-drv->head = 0;
-drv->track = 0;
-drv->sect = 1;
+fd_seek(drv, 0, 0, 1, 1);
 }
 
 /* Revalidate a disk drive after a disk change */
@@ -711,14 +713,6 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t 
status0)
 qemu_set_irq(fdctrl->irq, 1);
 fdctrl->sra |= FD_SRA_INTPEND;
 }
-if (status0 & FD_SR0_SEEK) {
-FDrive *cur_drv;
-/* A seek clears the disk change line (if a disk is inserted) */
-cur_drv = get_cur_drv(fdctrl);
-if (cur_drv->bs != NULL && bdrv_is_inserted(cur_drv->bs)) {
-cur_drv->media_changed = 0;
-}
-}
 
 fdctrl->reset_sensei = 0;
 fdctrl->status0 = status0;
@@ -997,7 +991,10 @@ static void fdctrl_unimplemented(FDCtrl *fdctrl, int 
direction)
 fdctrl_set_fifo(fdctrl, 1, 0);
 }
 
-/* Seek to next sector */
+/* Seek to next sector
+ * returns 0 when end of track reached (for DBL_SIDES on head 1)
+ * otherwise returns 1
+ */
 static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
 {
 FLOPPY_DPRINTF("seek to next sector (%d %02x %02x => %d)\n",
@@ -1005,30 +1002,39 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, 
FDrive *cur_drv)
fd_sector(cur_drv));
 /* XXX: cur_drv->sect >= cur_drv->last_sect should be an
error in fact */
-if (cur_drv->sect >= cur_drv->last_sect ||
-cur_drv->sect == fdctrl->eot) {
-cur_drv->sect = 1;
+uint8_t new_head = cur_drv->head;
+uint8_t new_track = cur_drv->track;
+uint8_t new_sect = cur_drv->sect;
+
+int ret = 1;
+
+if (new_sect >= cur_drv->last_sect ||
+new_sect == fdctrl->eot) {
+new_sect = 1;
 if (FD_MULTI_TRACK(fdctrl->data_state)) {
-if (cur_drv->head == 0 &&
+if (new_head == 0 &&
 (cur_drv->flags & FDISK_DBL_SIDES) != 0) {
-cur_drv->head = 1;
+new_head = 1;
 } else {
-cur_drv->head = 0;
-cur_drv->track++;
-if ((cur_drv->flags & FDISK_DBL_SIDES) == 0)
-return 0;
+new_head = 0;
+new_track++;
+if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) {
+ret = 0;
+}
 }
 } else {
-cur_drv->track++;
-return 0;
+new_track++;
+ret = 0;
+}
+if (ret == 1) {
+FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n",
+new_head, new_track, new_sect, fd_sector(cur_drv));
 }
-FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n",
-   cur_drv->head, cur_drv->track,
-   cur_drv->sect, fd_sector(cur_drv));
 } else {
-cur_drv->sect++;
+new_sect++;
 }
-return 1;
+fd_seek(cur_drv, new_head, new_track, new_sect, 1);
+return ret;
 }
 
 /* Callback for transfer end (stop or abort) */
@@ -1626,11 +1632,7 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int 
direction)
 /* The seek command just sends step pulses to the drive and doesn't care if
  * there is a medium inserted of if it's banging the head against the 
drive.
  */
-if (fdctrl->fifo[2] > cur_drv->max_track) {
-cur_drv->track = cur_drv->max_track;
-} else {
-cur_drv->track = fdctrl->fifo[2];
-}
+fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
 /* Raise Interrupt */
 fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
 }
@@ -1695,9 +1697,10 @@ static void fdctrl_handle_relative_seek_out(FDCtrl 
*fdctrl, int direction)
 SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
 cur_drv = get_cur_drv(fdctrl);
 if (fdctrl->fifo[2] + cur_drv->track >= cur_drv->max_track) {
-cur_drv->track = cur_drv->max_track - 1;
+fd_

[Qemu-devel] [PATCH 05/25] sheepdog: restart I/O when socket becomes ready in do_co_req()

2012-07-09 Thread Kevin Wolf
From: MORITA Kazutaka 

Currently, no one reenters the yielded coroutine.  This fixes it.

Signed-off-by: MORITA Kazutaka 
Signed-off-by: Kevin Wolf 
---
 block/sheepdog.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index afd06aa..0b49c6d 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -577,10 +577,21 @@ out:
 return ret;
 }
 
+static void restart_co_req(void *opaque)
+{
+Coroutine *co = opaque;
+
+qemu_coroutine_enter(co, NULL);
+}
+
 static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data,
   unsigned int *wlen, unsigned int *rlen)
 {
 int ret;
+Coroutine *co;
+
+co = qemu_coroutine_self();
+qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co);
 
 socket_set_block(sockfd);
 ret = send_co_req(sockfd, hdr, data, wlen);
@@ -588,6 +599,8 @@ static coroutine_fn int do_co_req(int sockfd, SheepdogReq 
*hdr, void *data,
 goto out;
 }
 
+qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, NULL, co);
+
 ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
 if (ret < sizeof(*hdr)) {
 error_report("failed to get a rsp, %s", strerror(errno));
@@ -609,6 +622,7 @@ static coroutine_fn int do_co_req(int sockfd, SheepdogReq 
*hdr, void *data,
 }
 ret = 0;
 out:
+qemu_aio_set_fd_handler(sockfd, NULL, NULL, NULL, NULL);
 socket_set_nonblock(sockfd);
 return ret;
 }
-- 
1.7.6.5




[Qemu-devel] [PATCH 04/25] sheepdog: fix dprintf format strings

2012-07-09 Thread Kevin Wolf
From: MORITA Kazutaka 

This fixes warnings about dprintf format in debug mode.

Signed-off-by: MORITA Kazutaka 
Signed-off-by: Kevin Wolf 
---
 block/sheepdog.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 8877f45..afd06aa 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1571,11 +1571,11 @@ static int coroutine_fn sd_co_rw_vector(void *p)
 }
 
 if (create) {
-dprintf("update ino (%" PRIu32") %" PRIu64 " %" PRIu64
-" %" PRIu64 "\n", inode->vdi_id, oid,
+dprintf("update ino (%" PRIu32 ") %" PRIu64 " %" PRIu64 " %ld\n",
+inode->vdi_id, oid,
 vid_to_data_oid(inode->data_vdi_id[idx], idx), idx);
 oid = vid_to_data_oid(inode->vdi_id, idx);
-dprintf("new oid %lx\n", oid);
+dprintf("new oid %" PRIx64 "\n", oid);
 }
 
 aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, old_oid, 
done);
@@ -1726,7 +1726,7 @@ static int sd_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 SheepdogInode *inode;
 unsigned int datalen;
 
-dprintf("sn_info: name %s id_str %s s: name %s vm_state_size %d "
+dprintf("sn_info: name %s id_str %s s: name %s vm_state_size %" PRId64 " "
 "is_snapshot %d\n", sn_info->name, sn_info->id_str,
 s->name, sn_info->vm_state_size, s->is_snapshot);
 
-- 
1.7.6.5




[Qemu-devel] [PULL 00/25] Block patches

2012-07-09 Thread Kevin Wolf
The following changes since commit 84988cf910a6881f2180fdcec516b60f8f0dc8c4:

  bitops.h: Add functions to extract and deposit bitfields (2012-07-07 09:07:01 
+)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git for-anthony

MORITA Kazutaka (6):
  sheepdog: fix dprintf format strings
  sheepdog: restart I/O when socket becomes ready in do_co_req()
  sheepdog: use coroutine based socket functions in coroutine context
  sheepdog: make sure we don't free aiocb before sending all requests
  sheepdog: split outstanding list into inflight and pending
  sheepdog: traverse pending_list from the first for each time

Markus Armbruster (4):
  fdc: Drop broken code for user-defined floppy geometry
  fdc: Move floppy geometry guessing back from block.c
  qtest: Tidy up temporary files properly
  block: Factor bdrv_read_unthrottled() out of guess_disk_lchs()

Paolo Bonzini (8):
  blkdebug: remove sync i/o events
  blkdebug: tiny cleanup
  blkdebug: pass getlength to underlying file
  blkdebug: store list of active rules
  blkdebug: optionally tie errors to a specific sector
  raw: hook into blkdebug
  block: copy over job and dirty bitmap fields in bdrv_append
  block: introduce bdrv_swap, implement bdrv_append on top of it

Pavel Hrdina (4):
  fdc: rewrite seek and DSKCHG bit handling
  fdc: fix interrupt handling
  fdc_test: update media_change test
  fdc_test: introduce test_sense_interrupt

Stefan Hajnoczi (3):
  qcow2: fix #ifdef'd qcow2_check_refcounts() callers
  qcow2: preserve free_byte_offset when qcow2_alloc_bytes() fails
  blockdev: warn when copy_on_read=on and readonly=on

 block.c|  306 +++-
 block.h|   23 +---
 block/blkdebug.c   |  107 ++---
 block/qcow2-refcount.c |7 +-
 block/qcow2-snapshot.c |6 +-
 block/qcow2.c  |2 +-
 block/qed.c|2 +-
 block/raw.c|2 +
 block/sheepdog.c   |  130 +
 blockdev.c |4 +
 hw/fdc.c   |  238 +++--
 hw/fdc.h   |   10 ++-
 hw/pc.c|   13 +--
 tests/fdc-test.c   |   50 +++--
 tests/libqtest.c   |   29 +++--
 15 files changed, 522 insertions(+), 407 deletions(-)



Re: [Qemu-devel] [PATCH] PPC: e500: reconditionalize on CONFIG_FDT

2012-07-09 Thread Andreas Färber
Am 09.07.2012 16:04, schrieb Scott Wood:
> Recent patches "PPC: e500: rename mpc8544ds into generic file", "PPC:
> e500: split mpc8544ds machine from generic e500 code", and "PPC: e500:
> add generic e500 platform" moved certain e500-related files to the bottom
> of the makefile because they're now in ppc/, but the dependency on
> CONFIG_FDT was accidentally dropped.  This broke the build when FDT
> support is not enabled.
> 
> Signed-off-by: Scott Wood 

Reviewed-by: Andreas Färber 

But this can probably still be squashed into the original patches for
bisectability I hope?

Andreas

> ---
>  hw/ppc/Makefile.objs |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 81bcc72..951e407 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -27,4 +27,4 @@ obj-y += xilinx_ethlite.o
>  
>  obj-y := $(addprefix ../,$(obj-y))
>  
> -obj-y += e500.o mpc8544ds.o e500plat.o
> +obj-$(CONFIG_FDT) += e500.o mpc8544ds.o e500plat.o
> 


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





[Qemu-devel] [PATCH v3 0/3] Add -cpu best support

2012-07-09 Thread Alexander Graf
[resend, forgot the v3, sorry]

This patch set implements support for a sane default CPU type, that plays the
middle ground between -cpu host (unmanagable test matrix) and -cpu qemu64 
(breaks
assumptions wrt family/model numbers). It also makes it the default for -M pc,
so that users who don't specify a specific CPU type on the command line always
get a fast, yet reliable CPU type chosen for them.

For a detailed description, please see patch 1/3.

Alexander Graf (3):
  KVM: Add new -cpu best
  KVM: Use -cpu best as default on x86
  i386: KVM: List -cpu host and best in -cpu ?

 hw/pc_piix.c  |   34 +-
 target-i386/cpu.c |   79 +++--
 2 files changed, 102 insertions(+), 11 deletions(-)




Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd

2012-07-09 Thread Kevin Wolf
Am 06.07.2012 19:40, schrieb Corey Bryant:
> 
> 
> On 07/06/2012 05:11 AM, Kevin Wolf wrote:
>> Am 05.07.2012 19:00, schrieb Eric Blake:
>>> On 07/05/2012 10:35 AM, Corey Bryant wrote:
 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with
 refcount of 0; fd=4's in-use flag is turned on
 2. client calls 'device-add' with /dev/fdset/1 as the backing filename,
 so qemu_open() increments the refcount of fdset1 to 1
 3. client calls 'remove-fd fdset=1 fd=4', so qemu marks fd=4 as no
 longer in-use by the monitor, and is left open because it is in use by
 the block device (refcount is 1)
 4. client crashes, so all tracked fds are visited; fd=4 is not in-use
 but refcount is 1 so it is not closed
>>> 5. client re-establishes QMP connection, so all tracked fds are visited.
>>>   What happens to the fd=4 in-use flag?
>>>
>>> ...but what are the semantics here?
>>>
>>> If we _always_ turn the in-use flag back on, then that says that even
>>> though libvirt successfully asked to close the fd, the reconnection
>>> means that libvirt once again has to ask to close things.
>>>
>>> If we _never_ turn the in-use flag back on, then we've broken the first
>>> case above where we want an in-use fd to come back into use after a crash.
>>>
>>> Maybe that argues for two flags per fd: an in-use flag (there is
>>> currently a monitor connection that can manipulate the fd, either
>>> because it passed in the fd or because it reconnected) and a removed
>>> flag (a monitor called remove-fd, and no longer wants to know about the
>>> fd, even if it crashes and reconnects).
>>
>> I was in fact just going to suggest a removed flag as well, however
>> combined with including the monitor connections in the refcount instead
>> of an additional flag. This would also allow to have (the currently
>> mostly hypothetical case of) multiple QMP monitors without interesting
>> things happening.
>>
>> Maybe I'm missing some point that the inuse flag would allow and
>> including monitors in the refcount doesn't. Is there one?
>>
>> Kevin
>>
> 
> Ok let me try this again. I was going through some of the examples and I 
> think we need a separate in-use flag.  Otherwise, when refcount gets to 
> 1, we don't know if it is because of a monitor reference or a block 
> device reference.  

Does it matter?

> I think it would cause fds to sit on the monitor 
> until refcount gets to zero (monitor disconnects). Here's an example 
> without the in-use flag:
> 
> 1. client calls 'add-fd', qemu is now tracking fd=4 in fdset1 with
> refcount of 1 (incremented because of monitor reference); fd=4's remove 
> flag is initialized to off
> 2. client calls 'device-add' with /dev/fdset/1 as the backing filename;
> qemu_open() increments the refcount of fdset1 to 2
> 3. client crashes, so all fdsets are visited; fd=4 had not yet been
> passed to 'remove-fd', so it's remove flag is off; refcount for fdset1 
> is decremented to 1; fd=4 is left open because it is still in use by the 
> block device (refcount is 1)
> 4. client re-establishes QMP connection, refcount for fdset1 is 
> incremented to 2; 'query-fds' lets client learn about fd=4 still being 
> open as part of fdset1
> 5. client calls 'remove-fd fdset=1 fd=4'; qemu turns on remove flag for 
> fd=4; but fd=4 remains open because refcount of fdset1 is 2

It also decreases the reference count because the monitor doesn't use it
any more.

> 6. qemu_close is called for fd=4; refcount for fdset1 is decremented to 
> 1; fd=4 remains open because monitor still references fdset1 (refcount 
> of fdset1 is 1)

So here the refcount becomes 0 and the fdset is closed.

> 7. Sometime later.. QMP disconnects; refcount for fdset is decremented 
> to zero; fd=4 is closed

The only question that is a bit unclear to me is whether a remove-fd on
one monitor drops the refcount only for this monitor or for all
monitors. However, both options can be implemented without additional
flags or counters.

Kevin



Re: [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd

2012-07-09 Thread Luiz Capitulino
On Thu, 05 Jul 2012 11:06:56 -0400
Corey Bryant  wrote:

> 
> 
> On 07/04/2012 04:09 AM, Kevin Wolf wrote:
> > Am 03.07.2012 20:21, schrieb Corey Bryant:
> >> On 07/03/2012 02:00 PM, Eric Blake wrote:
> >>> On 07/03/2012 11:46 AM, Corey Bryant wrote:
> >>>
> 
>  Yes, I think adding a +1 to the refcount for the monitor makes sense.
> 
>  I'm a bit unsure how to increment the refcount when a monitor reconnects
>  though.  Maybe it is as simple as adding a +1 to each fd's refcount when
>  the next QMP monitor connects.
> >>>
> >>> Or maybe delay a +1 until after a 'query-fds' - it is not until the
> >>> monitor has reconnected and learned what fds it should be aware of that
> >>> incrementing the refcount again makes sense.  But that would mean making
> >>> 'query-fds' track whether this is the first call since the monitor
> >>> reconnected, as it shouldn't normally increase refcounts.
> >>
> >> This doesn't sound ideal.
> >
> > Yes, it's less than ideal.
> >
> >>> The other alternative is that the monitor never re-increments a
> >>> refcount.  Once a monitor disconnects, that fd is lost to the monitor,
> >>> and a reconnected monitor must pass in a new fd to be re-associated with
> >>> the fdset.  In other words, the monitor's use of an fd is a one-way
> >>> operation, starting life in use but ending at the first disconnect or
> >>> remove-fd.
> >>
> >> I would vote for this 2nd alternative.  As long as we're not introducing
> >> an fd leak.  And I don't think we are if we decrement the refcount on
> >> remove-fd or on QMP disconnect.
> >
> > In fact, I believe this one is even worse. I can already see hacks like
> > adding a dummy FD with invalid flags and removing it again just to
> > regain control over the fdset...
> >
> > You earlier suggestion made a lot of sense to me: Whenever a new QMP
> > monitor is connected, increase the refcount. That is, as long as any
> > monitor is there, don't drop any fdsets unless explicitly requested via QMP.
> 
> Ok.  So refcount would be incremented (for the fd or fdset, whatever we 
> decide on) when QMP reconnects.  I'm assuming we wouldn't wait until 
> after a query-fds call.

I'm not sure this is a good idea because we will leak fds if the client forgets
about the fds when re-connecting (ie. it was restarted) or if a different
client connects to QMP.

If we really want to do that, I think that the right way of doing this is to
add a command for clients to re-again ownership of the fds on reconnection.

But to be honest, I don't fully understand why this is needed.



[Qemu-devel] [PATCH] PPC: e500: reconditionalize on CONFIG_FDT

2012-07-09 Thread Scott Wood
Recent patches "PPC: e500: rename mpc8544ds into generic file", "PPC:
e500: split mpc8544ds machine from generic e500 code", and "PPC: e500:
add generic e500 platform" moved certain e500-related files to the bottom
of the makefile because they're now in ppc/, but the dependency on
CONFIG_FDT was accidentally dropped.  This broke the build when FDT
support is not enabled.

Signed-off-by: Scott Wood 
---
 hw/ppc/Makefile.objs |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 81bcc72..951e407 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -27,4 +27,4 @@ obj-y += xilinx_ethlite.o
 
 obj-y := $(addprefix ../,$(obj-y))
 
-obj-y += e500.o mpc8544ds.o e500plat.o
+obj-$(CONFIG_FDT) += e500.o mpc8544ds.o e500plat.o
-- 
1.7.5.4




[Qemu-devel] [PATCH v2] configure: add -Werror to QEMU_CFLAGS early

2012-07-09 Thread Alexander Graf
We want all configure tests pass with -Werror if it is enabled. So we
need to update QEMU_CFLAGS early on to make sure we also pass it in to
all the compile test jobs.

This fixes a warning-became-error bug in nss for me with the default
configuration:

In file included from /usr/include/nss3/pkcs11t.h:1780,
 from /usr/include/nss3/keythi.h:41,
 from /usr/include/nss3/keyt.h:41,
 from /usr/include/nss3/pk11pub.h:43,
 from libcacard/vcard_emul_nss.c:21:
/usr/include/nss3/pkcs11n.h:365:26: error: "__GNUC_MINOR" is not defined

Signed-off-by: Alexander Graf 
Acked-by: Gerd Hoffmann 

---

v1 -> v2:

  - fix thinko in parameter list
---
 configure |   35 +--
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index 9f071b7..11a8b60 100755
--- a/configure
+++ b/configure
@@ -1141,10 +1141,26 @@ else
 exit 1
 fi
 
+# Consult white-list to determine whether to enable werror
+# by default.  Only enable by default for git builds
+z_version=`cut -f3 -d. $source_path/VERSION`
+
+if test -z "$werror" ; then
+if test "$z_version" = "50" -a \
+"$linux" = "yes" ; then
+werror="yes"
+else
+werror="no"
+fi
+fi
+
 gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
 gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers 
$gcc_flags"
 gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
 gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
+if test "$werror" = "yes" ; then
+gcc_flags="-Werror $gcc_flags"
+fi
 cat > $TMPC << EOF
 int main(void) { return 0; }
 EOF
@@ -2575,7 +2591,7 @@ if test "$libiscsi" != "no" ; then
 #include 
 int main(void) { iscsi_unmap_sync(NULL,0,0,0,NULL,0); return 0; }
 EOF
-  if compile_prog "-Werror" "-liscsi" ; then
+  if compile_prog "" "-liscsi" ; then
 libiscsi="yes"
 LIBS="$LIBS -liscsi"
   else
@@ -2879,19 +2895,6 @@ if test "$debug" = "no" ; then
   CFLAGS="-O2 -D_FORTIFY_SOURCE=2 $CFLAGS"
 fi
 
-# Consult white-list to determine whether to enable werror
-# by default.  Only enable by default for git builds
-z_version=`cut -f3 -d. $source_path/VERSION`
-
-if test -z "$werror" ; then
-if test "$z_version" = "50" -a \
-"$linux" = "yes" ; then
-werror="yes"
-else
-werror="no"
-fi
-fi
-
 # Disable zero malloc errors for official releases unless explicitly told to
 # enable/disable
 if test -z "$zero_malloc" ; then
@@ -2902,10 +2905,6 @@ if test -z "$zero_malloc" ; then
 fi
 fi
 
-if test "$werror" = "yes" ; then
-QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
-fi
-
 if test "$solaris" = "no" ; then
 if $ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
 LDFLAGS="-Wl,--warn-common $LDFLAGS"
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH 2/3] KVM: Use -cpu best as default on x86

2012-07-09 Thread Eric Blake
On 07/09/2012 07:48 AM, Alexander Graf wrote:
> 
> On 09.07.2012, at 15:47, Eric Blake wrote:
> 
>> On 07/09/2012 06:10 AM, Alexander Graf wrote:
>>
>>>
>>> This fixes a lot of subtle breakage in the GNU toolchain (libgmp) which
>>> hicks up on QEMU's non-existent CPU models.
>>
>> s/hicks up/hiccups/
>>
>>
>>>
>>> v2 -> v3:
>>>
>>>  - fix typo in commit message
>>
>> but not all of them :)
> 
> I hope you don't expect me to respin now :)

I don't.  If someone wants to touch it up, great; if not, it's not the
first typo in a commit.  :)

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





signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 2/3] KVM: Use -cpu best as default on x86

2012-07-09 Thread Alexander Graf

On 09.07.2012, at 15:47, Eric Blake wrote:

> On 07/09/2012 06:10 AM, Alexander Graf wrote:
> 
>> 
>> This fixes a lot of subtle breakage in the GNU toolchain (libgmp) which
>> hicks up on QEMU's non-existent CPU models.
> 
> s/hicks up/hiccups/
> 
> 
>> 
>> v2 -> v3:
>> 
>>  - fix typo in commit message
> 
> but not all of them :)

I hope you don't expect me to respin now :)


Alex




Re: [Qemu-devel] [PATCH 2/3] KVM: Use -cpu best as default on x86

2012-07-09 Thread Eric Blake
On 07/09/2012 06:10 AM, Alexander Graf wrote:

> 
> This fixes a lot of subtle breakage in the GNU toolchain (libgmp) which
> hicks up on QEMU's non-existent CPU models.

s/hicks up/hiccups/


> 
> v2 -> v3:
> 
>   - fix typo in commit message

but not all of them :)

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





signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v3 1/3] KVM: Add new -cpu best

2012-07-09 Thread Alexander Graf
During discussions on whether to support -cpu host in SLE, I found myself
disagreeing to the thought, because it potentially opens a big can of worms
for potential bugs. But if I already am so opposed to it for SLE, how can
it possibly be reasonable to default to -cpu host in upstream QEMU? And what
would a sane default look like?

So I had this idea of looping through all available CPU definitions. We can
pretty well tell if our host is able to execute any of them by checking the
respective flags and seeing if our host has all features the CPU definition
requires. With that, we can create a -cpu type that would fall back to the
"best known CPU definition" that our host can fulfill. On my Phenom II
system for example, that would be -cpu phenom.

With this approach we can test and verify that CPU types actually work at
any random user setup, because we can always verify that all the -cpu types
we ship actually work. And we only default to some clever mechanism that
chooses from one of these.

Signed-off-by: Alexander Graf 

---

v2 -> v3:

  - clarify commit message
  - fix avi's style comments
---
 target-i386/cpu.c |   72 +
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5521709..a5d9468 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -558,6 +558,76 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
 return 0;
 }
 
+/* Are all guest feature bits present on the host? */
+static bool cpu_x86_feature_subset(uint32_t host, uint32_t guest)
+{
+return !(guest & ~host);
+}
+
+/* Does the host support all the features of the CPU definition? */
+static bool cpu_x86_fits_host(x86_def_t *x86_cpu_def)
+{
+uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
+
+host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
+if (x86_cpu_def->level > eax) {
+return false;
+}
+if ((x86_cpu_def->vendor1 != ebx) ||
+(x86_cpu_def->vendor2 != edx) ||
+(x86_cpu_def->vendor3 != ecx)) {
+return false;
+}
+
+host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
+if (!cpu_x86_feature_subset(ecx, x86_cpu_def->ext_features) ||
+!cpu_x86_feature_subset(edx, x86_cpu_def->features)) {
+return false;
+}
+
+host_cpuid(0x8000, 0, &eax, &ebx, &ecx, &edx);
+if (x86_cpu_def->xlevel > eax) {
+return false;
+}
+
+host_cpuid(0x8001, 0, &eax, &ebx, &ecx, &edx);
+if (!cpu_x86_feature_subset(edx, x86_cpu_def->ext2_features) ||
+!cpu_x86_feature_subset(ecx, x86_cpu_def->ext3_features)) {
+return false;
+}
+
+return true;
+}
+
+/* Returns true when new_def is higher versioned than old_def */
+static int cpu_x86_fits_higher(x86_def_t *new_def, x86_def_t *old_def)
+{
+int old_fammod = (old_def->family << 24) | (old_def->model << 8)
+   | (old_def->stepping);
+int new_fammod = (new_def->family << 24) | (new_def->model << 8)
+   | (new_def->stepping);
+
+return new_fammod > old_fammod;
+}
+
+static void cpu_x86_fill_best(x86_def_t *x86_cpu_def)
+{
+x86_def_t *def;
+
+x86_cpu_def->family = 0;
+x86_cpu_def->model = 0;
+for (def = x86_defs; def; def = def->next) {
+if (cpu_x86_fits_host(def) && cpu_x86_fits_higher(def, x86_cpu_def)) {
+*x86_cpu_def = *def;
+}
+}
+
+if (!x86_cpu_def->family && !x86_cpu_def->model) {
+fprintf(stderr, "No fitting CPU model found!\n");
+exit(1);
+}
+}
+
 static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
 {
 int i;
@@ -878,6 +948,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 break;
 if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 cpu_x86_fill_host(x86_cpu_def);
+} else if (kvm_enabled() && name && strcmp(name, "best") == 0) {
+cpu_x86_fill_best(x86_cpu_def);
 } else if (!def) {
 goto error;
 } else {
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH] configure: add -Werror to QEMU_CFLAGS early

2012-07-09 Thread Gerd Hoffmann
On 07/09/12 14:23, Alexander Graf wrote:
> We want all configure tests pass with -Werror if it is enabled. So we
> need to update QEMU_CFLAGS early on to make sure we also pass it in to
> all the compile test jobs.

Much better than v1

Acked-by: Gerd Hoffmann 

cheers,
  Gerd



[Qemu-devel] [PATCH] configure: add -Werror to QEMU_CFLAGS early

2012-07-09 Thread Alexander Graf
We want all configure tests pass with -Werror if it is enabled. So we
need to update QEMU_CFLAGS early on to make sure we also pass it in to
all the compile test jobs.

This fixes a warning-became-error bug in nss for me with the default
configuration:

In file included from /usr/include/nss3/pkcs11t.h:1780,
 from /usr/include/nss3/keythi.h:41,
 from /usr/include/nss3/keyt.h:41,
 from /usr/include/nss3/pk11pub.h:43,
 from libcacard/vcard_emul_nss.c:21:
/usr/include/nss3/pkcs11n.h:365:26: error: "__GNUC_MINOR" is not defined

Signed-off-by: Alexander Graf 
---
 configure |   35 +--
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index 9f071b7..fb67c53 100755
--- a/configure
+++ b/configure
@@ -1141,10 +1141,26 @@ else
 exit 1
 fi
 
+# Consult white-list to determine whether to enable werror
+# by default.  Only enable by default for git builds
+z_version=`cut -f3 -d. $source_path/VERSION`
+
+if test -z "$werror" ; then
+if test "$z_version" = "50" -a \
+"$linux" = "yes" ; then
+werror="yes"
+else
+werror="no"
+fi
+fi
+
 gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
 gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers 
$gcc_flags"
 gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
 gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
+if test "$werror" = "yes" ; then
+gcc_flags="gcc_flags -Werror $gcc_flags"
+fi
 cat > $TMPC << EOF
 int main(void) { return 0; }
 EOF
@@ -2575,7 +2591,7 @@ if test "$libiscsi" != "no" ; then
 #include 
 int main(void) { iscsi_unmap_sync(NULL,0,0,0,NULL,0); return 0; }
 EOF
-  if compile_prog "-Werror" "-liscsi" ; then
+  if compile_prog "" "-liscsi" ; then
 libiscsi="yes"
 LIBS="$LIBS -liscsi"
   else
@@ -2879,19 +2895,6 @@ if test "$debug" = "no" ; then
   CFLAGS="-O2 -D_FORTIFY_SOURCE=2 $CFLAGS"
 fi
 
-# Consult white-list to determine whether to enable werror
-# by default.  Only enable by default for git builds
-z_version=`cut -f3 -d. $source_path/VERSION`
-
-if test -z "$werror" ; then
-if test "$z_version" = "50" -a \
-"$linux" = "yes" ; then
-werror="yes"
-else
-werror="no"
-fi
-fi
-
 # Disable zero malloc errors for official releases unless explicitly told to
 # enable/disable
 if test -z "$zero_malloc" ; then
@@ -2902,10 +2905,6 @@ if test -z "$zero_malloc" ; then
 fi
 fi
 
-if test "$werror" = "yes" ; then
-QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
-fi
-
 if test "$solaris" = "no" ; then
 if $ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
 LDFLAGS="-Wl,--warn-common $LDFLAGS"
-- 
1.6.0.2




[Qemu-devel] [PATCH 1/3] KVM: Add new -cpu best

2012-07-09 Thread Alexander Graf
During discussions on whether to support -cpu host in SLE, I found myself
disagreeing to the thought, because it potentially opens a big can of worms
for potential bugs. But if I already am so opposed to it for SLE, how can
it possibly be reasonable to default to -cpu host in upstream QEMU? And what
would a sane default look like?

So I had this idea of looping through all available CPU definitions. We can
pretty well tell if our host is able to execute any of them by checking the
respective flags and seeing if our host has all features the CPU definition
requires. With that, we can create a -cpu type that would fall back to the
"best known CPU definition" that our host can fulfill. On my Phenom II
system for example, that would be -cpu phenom.

With this approach we can test and verify that CPU types actually work at
any random user setup, because we can always verify that all the -cpu types
we ship actually work. And we only default to some clever mechanism that
chooses from one of these.

Signed-off-by: Alexander Graf 

---

v2 -> v3:

  - clarify commit message
  - fix avi's style comments
---
 target-i386/cpu.c |   72 +
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5521709..a5d9468 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -558,6 +558,76 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
 return 0;
 }
 
+/* Are all guest feature bits present on the host? */
+static bool cpu_x86_feature_subset(uint32_t host, uint32_t guest)
+{
+return !(guest & ~host);
+}
+
+/* Does the host support all the features of the CPU definition? */
+static bool cpu_x86_fits_host(x86_def_t *x86_cpu_def)
+{
+uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
+
+host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
+if (x86_cpu_def->level > eax) {
+return false;
+}
+if ((x86_cpu_def->vendor1 != ebx) ||
+(x86_cpu_def->vendor2 != edx) ||
+(x86_cpu_def->vendor3 != ecx)) {
+return false;
+}
+
+host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
+if (!cpu_x86_feature_subset(ecx, x86_cpu_def->ext_features) ||
+!cpu_x86_feature_subset(edx, x86_cpu_def->features)) {
+return false;
+}
+
+host_cpuid(0x8000, 0, &eax, &ebx, &ecx, &edx);
+if (x86_cpu_def->xlevel > eax) {
+return false;
+}
+
+host_cpuid(0x8001, 0, &eax, &ebx, &ecx, &edx);
+if (!cpu_x86_feature_subset(edx, x86_cpu_def->ext2_features) ||
+!cpu_x86_feature_subset(ecx, x86_cpu_def->ext3_features)) {
+return false;
+}
+
+return true;
+}
+
+/* Returns true when new_def is higher versioned than old_def */
+static int cpu_x86_fits_higher(x86_def_t *new_def, x86_def_t *old_def)
+{
+int old_fammod = (old_def->family << 24) | (old_def->model << 8)
+   | (old_def->stepping);
+int new_fammod = (new_def->family << 24) | (new_def->model << 8)
+   | (new_def->stepping);
+
+return new_fammod > old_fammod;
+}
+
+static void cpu_x86_fill_best(x86_def_t *x86_cpu_def)
+{
+x86_def_t *def;
+
+x86_cpu_def->family = 0;
+x86_cpu_def->model = 0;
+for (def = x86_defs; def; def = def->next) {
+if (cpu_x86_fits_host(def) && cpu_x86_fits_higher(def, x86_cpu_def)) {
+*x86_cpu_def = *def;
+}
+}
+
+if (!x86_cpu_def->family && !x86_cpu_def->model) {
+fprintf(stderr, "No fitting CPU model found!\n");
+exit(1);
+}
+}
+
 static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
 {
 int i;
@@ -878,6 +948,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 break;
 if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 cpu_x86_fill_host(x86_cpu_def);
+} else if (kvm_enabled() && name && strcmp(name, "best") == 0) {
+cpu_x86_fill_best(x86_cpu_def);
 } else if (!def) {
 goto error;
 } else {
-- 
1.6.0.2




[Qemu-devel] [PATCH v3 2/3] KVM: Use -cpu best as default on x86

2012-07-09 Thread Alexander Graf
When running QEMU without -cpu parameter, the user usually wants a sane
default. So far, we're using the qemu64/qemu32 CPU type, which basically
means "the maximum TCG can emulate".

That's a really good default when using TCG, but when running with KVM
we much rather want a default saying "the maximum performance I can get".

Fortunately we just added an option that gives us the best performance
while still staying safe on the testability side of things: -cpu best.
So all we need to do is make -cpu best the default when the user doesn't
explicitly specify a CPU type.

This fixes a lot of subtle breakage in the GNU toolchain (libgmp) which
hicks up on QEMU's non-existent CPU models.

This patch also adds a new pc-1.2 machine type to stay backwards compatible
with older versions of QEMU.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

  - rebase

v2 -> v3:

  - fix typo in commit message
---
 hw/pc_piix.c |   34 ++
 1 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0c0096f..a955bf8 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -127,7 +127,8 @@ static void pc_init1(MemoryRegion *system_memory,
  const char *initrd_filename,
  const char *cpu_model,
  int pci_enabled,
- int kvmclock_enabled)
+ int kvmclock_enabled,
+ int may_cpu_best)
 {
 int i;
 ram_addr_t below_4g_mem_size, above_4g_mem_size;
@@ -149,6 +150,9 @@ static void pc_init1(MemoryRegion *system_memory,
 MemoryRegion *rom_memory;
 void *fw_cfg = NULL;
 
+if (!cpu_model && kvm_enabled() && may_cpu_best) {
+cpu_model = "best";
+}
 pc_cpus_init(cpu_model);
 
 if (kvmclock_enabled) {
@@ -298,7 +302,21 @@ static void pc_init_pci(ram_addr_t ram_size,
  get_system_io(),
  ram_size, boot_device,
  kernel_filename, kernel_cmdline,
- initrd_filename, cpu_model, 1, 1);
+ initrd_filename, cpu_model, 1, 1, 1);
+}
+
+static void pc_init_pci_oldcpu(ram_addr_t ram_size,
+   const char *boot_device,
+   const char *kernel_filename,
+   const char *kernel_cmdline,
+   const char *initrd_filename,
+   const char *cpu_model)
+{
+pc_init1(get_system_memory(),
+ get_system_io(),
+ ram_size, boot_device,
+ kernel_filename, kernel_cmdline,
+ initrd_filename, cpu_model, 1, 1, 0);
 }
 
 static void pc_init_pci_no_kvmclock(ram_addr_t ram_size,
@@ -312,7 +330,7 @@ static void pc_init_pci_no_kvmclock(ram_addr_t ram_size,
  get_system_io(),
  ram_size, boot_device,
  kernel_filename, kernel_cmdline,
- initrd_filename, cpu_model, 1, 0);
+ initrd_filename, cpu_model, 1, 0, 0);
 }
 
 static void pc_init_isa(ram_addr_t ram_size,
@@ -328,7 +346,7 @@ static void pc_init_isa(ram_addr_t ram_size,
  get_system_io(),
  ram_size, boot_device,
  kernel_filename, kernel_cmdline,
- initrd_filename, cpu_model, 0, 1);
+ initrd_filename, cpu_model, 0, 1, 0);
 }
 
 #ifdef CONFIG_XEN
@@ -380,7 +398,7 @@ static QEMUMachine pc_machine_v1_2 = {
 static QEMUMachine pc_machine_v1_1 = {
 .name = "pc-1.1",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_1_1,
@@ -415,7 +433,7 @@ static QEMUMachine pc_machine_v1_1 = {
 static QEMUMachine pc_machine_v1_0 = {
 .name = "pc-1.0",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_1_0,
@@ -430,7 +448,7 @@ static QEMUMachine pc_machine_v1_0 = {
 static QEMUMachine pc_machine_v0_15 = {
 .name = "pc-0.15",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_0_15,
@@ -462,7 +480,7 @@ static QEMUMachine pc_machine_v0_15 = {
 static QEMUMachine pc_machine_v0_14 = {
 .name = "pc-0.14",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_0_14, 
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH] configure: fix libsmartcard_nss compile check

2012-07-09 Thread Alexander Graf

On 09.07.2012, at 15:29, Gerd Hoffmann wrote:

> On 07/09/12 14:06, Alexander Graf wrote:
>> When just calling ./configure on my box, I always ran into the same issue:
>> 
>> In file included from /usr/include/nss3/pkcs11t.h:1780,
>> from /usr/include/nss3/keythi.h:41,
>> from /usr/include/nss3/keyt.h:41,
>> from /usr/include/nss3/pk11pub.h:43,
>> from libcacard/vcard_emul_nss.c:21:
>> /usr/include/nss3/pkcs11n.h:365:26: error: "__GNUC_MINOR" is not defined
>> 
>> This is a bug in nss3. But why didn't configure bail out of the feature
>> if it doesn't even compile? Because this really is just a warning, not an
>> error. But configure builds its test program without -Werror, while we do
>> build our sources with -Werror by default.
>> 
>> Force the check to also use -Werror. This fixes the default build for me 
>> again.
> 
> Good idea, but shouldn't we do that
>  (a) for all compile tests and
>  (b) only with --enable-werror

Yeah, my configure foo wasn't good enough to figure out when exactly -Werror is 
passed in. At the point in time when the smartcard check is done, neither 
$werror nor $QEMU_CFLAGS have been set.


Alex




Re: [Qemu-devel] [PATCH] configure: fix libsmartcard_nss compile check

2012-07-09 Thread Gerd Hoffmann
On 07/09/12 14:06, Alexander Graf wrote:
> When just calling ./configure on my box, I always ran into the same issue:
> 
> In file included from /usr/include/nss3/pkcs11t.h:1780,
>  from /usr/include/nss3/keythi.h:41,
>  from /usr/include/nss3/keyt.h:41,
>  from /usr/include/nss3/pk11pub.h:43,
>  from libcacard/vcard_emul_nss.c:21:
> /usr/include/nss3/pkcs11n.h:365:26: error: "__GNUC_MINOR" is not defined
> 
> This is a bug in nss3. But why didn't configure bail out of the feature
> if it doesn't even compile? Because this really is just a warning, not an
> error. But configure builds its test program without -Werror, while we do
> build our sources with -Werror by default.
> 
> Force the check to also use -Werror. This fixes the default build for me 
> again.

Good idea, but shouldn't we do that
  (a) for all compile tests and
  (b) only with --enable-werror

?

cheers,
  Gerd




[Qemu-devel] [PATCH v3 3/3] i386: KVM: List -cpu host and best in -cpu ?

2012-07-09 Thread Alexander Graf
The kvm_enabled() helper doesn't work in a function as early as -cpu ?
yet. It also doesn't make sense to list the -cpu ? output conditional on
the -enable-kvm parameter. So let's always mention -cpu host in the
CPU list when KVM is supported on that configuration.

In addition, this patch also adds listing of -cpu best in the -cpu ?
list, so that people know that this option exists.

Signed-off-by: Alexander Graf 
---
 target-i386/cpu.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a5d9468..c77 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1190,9 +1190,10 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, 
const char *optarg)
 (*cpu_fprintf)(f, "\n");
 }
 }
-if (kvm_enabled()) {
-(*cpu_fprintf)(f, "x86 %16s\n", "[host]");
-}
+#ifdef CONFIG_KVM
+(*cpu_fprintf)(f, "x86 %16s\n", "KVM only: [host]");
+(*cpu_fprintf)(f, "x86 %16s\n", "KVM only: [best]");
+#endif
 }
 
 int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH] configure: fix libsmartcard_nss compile check

2012-07-09 Thread Andreas Färber
Am 09.07.2012 14:06, schrieb Alexander Graf:
> When just calling ./configure on my box, I always ran into the same issue:
> 
> In file included from /usr/include/nss3/pkcs11t.h:1780,
>  from /usr/include/nss3/keythi.h:41,
>  from /usr/include/nss3/keyt.h:41,
>  from /usr/include/nss3/pk11pub.h:43,
>  from libcacard/vcard_emul_nss.c:21:
> /usr/include/nss3/pkcs11n.h:365:26: error: "__GNUC_MINOR" is not defined
> 
> This is a bug in nss3. But why didn't configure bail out of the feature
> if it doesn't even compile? Because this really is just a warning, not an
> error. But configure builds its test program without -Werror, while we do
> build our sources with -Werror by default.
> 
> Force the check to also use -Werror. This fixes the default build for me 
> again.
> 
> Signed-off-by: Alexander Graf 
> ---
>  configure |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index 9f071b7..d28a909 100755
> --- a/configure
> +++ b/configure
> @@ -2642,6 +2642,10 @@ EOF
>  smartcard_cflags="-I\$(SRC_PATH)/libcacard"
>  libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
>  libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) 
> $glib_cflags"
> +# nss < 3.13.3 has a nasty warning that can trigger compile failures
> +# with -Werror. Unfortunately, we don't know if -Werror is goint to

"going"

> +# be used here yet, so let's just assume it to be safe.

Maybe you can just temporarily supply it to compile_prog below then?

Andreas

> +libcacard_cflags="$libcacard_cflags -Werror"
>  if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
>compile_prog "$smartcard_cflags $libcacard_cflags" 
> "$libcacard_libs"; then
>  smartcard_nss="yes"
> 


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





[Qemu-devel] [PATCH 0/3] Add -cpu best support

2012-07-09 Thread Alexander Graf
This patch set implements support for a sane default CPU type, that plays the
middle ground between -cpu host (unmanagable test matrix) and -cpu qemu64 
(breaks
assumptions wrt family/model numbers). It also makes it the default for -M pc,
so that users who don't specify a specific CPU type on the command line always
get a fast, yet reliable CPU type chosen for them.

For a detailed description, please see patch 1/3.

Alexander Graf (3):
  KVM: Add new -cpu best
  KVM: Use -cpu best as default on x86
  i386: KVM: List -cpu host and best in -cpu ?

 hw/pc_piix.c  |   34 +-
 target-i386/cpu.c |   79 +++--
 2 files changed, 102 insertions(+), 11 deletions(-)




[Qemu-devel] [PATCH 2/3] KVM: Use -cpu best as default on x86

2012-07-09 Thread Alexander Graf
When running QEMU without -cpu parameter, the user usually wants a sane
default. So far, we're using the qemu64/qemu32 CPU type, which basically
means "the maximum TCG can emulate".

That's a really good default when using TCG, but when running with KVM
we much rather want a default saying "the maximum performance I can get".

Fortunately we just added an option that gives us the best performance
while still staying safe on the testability side of things: -cpu best.
So all we need to do is make -cpu best the default when the user doesn't
explicitly specify a CPU type.

This fixes a lot of subtle breakage in the GNU toolchain (libgmp) which
hicks up on QEMU's non-existent CPU models.

This patch also adds a new pc-1.2 machine type to stay backwards compatible
with older versions of QEMU.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

  - rebase

v2 -> v3:

  - fix typo in commit message
---
 hw/pc_piix.c |   34 ++
 1 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0c0096f..a955bf8 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -127,7 +127,8 @@ static void pc_init1(MemoryRegion *system_memory,
  const char *initrd_filename,
  const char *cpu_model,
  int pci_enabled,
- int kvmclock_enabled)
+ int kvmclock_enabled,
+ int may_cpu_best)
 {
 int i;
 ram_addr_t below_4g_mem_size, above_4g_mem_size;
@@ -149,6 +150,9 @@ static void pc_init1(MemoryRegion *system_memory,
 MemoryRegion *rom_memory;
 void *fw_cfg = NULL;
 
+if (!cpu_model && kvm_enabled() && may_cpu_best) {
+cpu_model = "best";
+}
 pc_cpus_init(cpu_model);
 
 if (kvmclock_enabled) {
@@ -298,7 +302,21 @@ static void pc_init_pci(ram_addr_t ram_size,
  get_system_io(),
  ram_size, boot_device,
  kernel_filename, kernel_cmdline,
- initrd_filename, cpu_model, 1, 1);
+ initrd_filename, cpu_model, 1, 1, 1);
+}
+
+static void pc_init_pci_oldcpu(ram_addr_t ram_size,
+   const char *boot_device,
+   const char *kernel_filename,
+   const char *kernel_cmdline,
+   const char *initrd_filename,
+   const char *cpu_model)
+{
+pc_init1(get_system_memory(),
+ get_system_io(),
+ ram_size, boot_device,
+ kernel_filename, kernel_cmdline,
+ initrd_filename, cpu_model, 1, 1, 0);
 }
 
 static void pc_init_pci_no_kvmclock(ram_addr_t ram_size,
@@ -312,7 +330,7 @@ static void pc_init_pci_no_kvmclock(ram_addr_t ram_size,
  get_system_io(),
  ram_size, boot_device,
  kernel_filename, kernel_cmdline,
- initrd_filename, cpu_model, 1, 0);
+ initrd_filename, cpu_model, 1, 0, 0);
 }
 
 static void pc_init_isa(ram_addr_t ram_size,
@@ -328,7 +346,7 @@ static void pc_init_isa(ram_addr_t ram_size,
  get_system_io(),
  ram_size, boot_device,
  kernel_filename, kernel_cmdline,
- initrd_filename, cpu_model, 0, 1);
+ initrd_filename, cpu_model, 0, 1, 0);
 }
 
 #ifdef CONFIG_XEN
@@ -380,7 +398,7 @@ static QEMUMachine pc_machine_v1_2 = {
 static QEMUMachine pc_machine_v1_1 = {
 .name = "pc-1.1",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_1_1,
@@ -415,7 +433,7 @@ static QEMUMachine pc_machine_v1_1 = {
 static QEMUMachine pc_machine_v1_0 = {
 .name = "pc-1.0",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_1_0,
@@ -430,7 +448,7 @@ static QEMUMachine pc_machine_v1_0 = {
 static QEMUMachine pc_machine_v0_15 = {
 .name = "pc-0.15",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_0_15,
@@ -462,7 +480,7 @@ static QEMUMachine pc_machine_v0_15 = {
 static QEMUMachine pc_machine_v0_14 = {
 .name = "pc-0.14",
 .desc = "Standard PC",
-.init = pc_init_pci,
+.init = pc_init_pci_oldcpu,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
 PC_COMPAT_0_14, 
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH] disas: Fix printing of addresses in disassembly

2012-07-09 Thread Peter Maydell
On 9 July 2012 14:19, Andreas Färber  wrote:
> Am 25.06.2012 16:55, schrieb Peter Maydell:
>> In our disassembly code, the bfd_vma type is always 64 bits,
>> even if the target's virtual address width is only 32 bits. This
>> means that when we print out addresses we need to truncate them
>> to 32 bits, to avoid odd output which has incorrectly sign-extended
>> a value to 64 bits, for instance this ARM example:
>> 0x80479a60:  e59f4088 ldr  r4, [pc, #136]  ; 0x80479a4f
>>
>> (It would also be possible to truncate before passing the address
>> to info->print_address_func(), but truncating in the final print
>> function is the same approach that binutils takes to this problem.)

>> +/* Print address in hex, truncated to the width of a target virtual 
>> address. */
>> +static void
>> +generic_print_target_address(bfd_vma addr, struct disassemble_info *info)
>> +{
>> +uint64_t mask = ~0ULL >> (64 - TARGET_VIRT_ADDR_SPACE_BITS);
>> +generic_print_address(addr & mask, info);
>> +}

> I wonder if TARGET_VIRT_ADDR_SPACE_BITS is the correct factor to use
> here though? Might sizeof(target_phys_addr_t) * 8 be safer?

That will give the wrong answer for ARM (when my LPAE patchset lands):
ARM virtual addresses are 32 bits but sizeof(target_phys_addr_t) * 8
will be 64.

> I'm thinking
> of the possibility of having an alias in the 64-bit address space point
> into the actual 36/48/... virtual address space. I have a ppc64 ld
> instruction in mind, for which a full 64-bit register would be set up
> that could not fully be represented in the virtual address space.

It would be helpful to check how the ppc disassembler handles that
ld insn. It may well either (a) not print the resulting address at
all or (b) print it itself. However, if the resulting actual address
is a virtual address space then the right thing to print would be
the truncated version, I think, assuming that is what the hardware will
actually do the load from.

-- PMM



[Qemu-devel] [PATCH] configure: fix libsmartcard_nss compile check

2012-07-09 Thread Alexander Graf
When just calling ./configure on my box, I always ran into the same issue:

In file included from /usr/include/nss3/pkcs11t.h:1780,
 from /usr/include/nss3/keythi.h:41,
 from /usr/include/nss3/keyt.h:41,
 from /usr/include/nss3/pk11pub.h:43,
 from libcacard/vcard_emul_nss.c:21:
/usr/include/nss3/pkcs11n.h:365:26: error: "__GNUC_MINOR" is not defined

This is a bug in nss3. But why didn't configure bail out of the feature
if it doesn't even compile? Because this really is just a warning, not an
error. But configure builds its test program without -Werror, while we do
build our sources with -Werror by default.

Force the check to also use -Werror. This fixes the default build for me again.

Signed-off-by: Alexander Graf 
---
 configure |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 9f071b7..d28a909 100755
--- a/configure
+++ b/configure
@@ -2642,6 +2642,10 @@ EOF
 smartcard_cflags="-I\$(SRC_PATH)/libcacard"
 libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
 libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
+# nss < 3.13.3 has a nasty warning that can trigger compile failures
+# with -Werror. Unfortunately, we don't know if -Werror is goint to
+# be used here yet, so let's just assume it to be safe.
+libcacard_cflags="$libcacard_cflags -Werror"
 if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
   compile_prog "$smartcard_cflags $libcacard_cflags" 
"$libcacard_libs"; then
 smartcard_nss="yes"
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH] flatload: fix bss clearing

2012-07-09 Thread Andreas Färber
Hi Mike,

Am 09.07.2012 15:04, schrieb Mike Frysinger:
> The current bss clear logic assumes the target mmap address and host
> address are the same.  Use g2h to translate from the target address
> space to the host so we can call memset on it.
> 
> Signed-off-by: Mike Frysinger 
> ---
>  linux-user/flatload.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Patch looks sensible. Are you working on rebasing your Blackfin target
to QOM and AREG0?

Cheers,
Andreas

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





Re: [Qemu-devel] [PATCH] disas: Fix printing of addresses in disassembly

2012-07-09 Thread Andreas Färber
Am 25.06.2012 16:55, schrieb Peter Maydell:
> In our disassembly code, the bfd_vma type is always 64 bits,
> even if the target's virtual address width is only 32 bits. This
> means that when we print out addresses we need to truncate them
> to 32 bits, to avoid odd output which has incorrectly sign-extended
> a value to 64 bits, for instance this ARM example:
> 0x80479a60:  e59f4088 ldr  r4, [pc, #136]  ; 0x80479a4f
> 
> (It would also be possible to truncate before passing the address
> to info->print_address_func(), but truncating in the final print
> function is the same approach that binutils takes to this problem.)
> 
> Signed-off-by: Peter Maydell 
> ---
>  disas.c |   19 +++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/disas.c b/disas.c
> index 93d8d30..7b2acc9 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -64,6 +64,22 @@ generic_print_address (bfd_vma addr, struct 
> disassemble_info *info)
>  (*info->fprintf_func) (info->stream, "0x%" PRIx64, addr);
>  }
>  
> +/* Print address in hex, truncated to the width of a target virtual address. 
> */
> +static void
> +generic_print_target_address(bfd_vma addr, struct disassemble_info *info)
> +{
> +uint64_t mask = ~0ULL >> (64 - TARGET_VIRT_ADDR_SPACE_BITS);
> +generic_print_address(addr & mask, info);
> +}
> +
> +/* Print address in hex, truncated to the width of a host virtual address. */
> +static void
> +generic_print_host_address(bfd_vma addr, struct disassemble_info *info)
> +{
> +uint64_t mask = ~0ULL >> (64 - (sizeof(void *) * 8));
> +generic_print_address(addr & mask, info);
> +}
> +
>  /* Just return the given address.  */
>  
>  int

As usual the inversion and subtracted shift are a bit confusing at
first, but the algorithm looks okay.

I wonder if TARGET_VIRT_ADDR_SPACE_BITS is the correct factor to use
here though? Might sizeof(target_phys_addr_t) * 8 be safer? I'm thinking
of the possibility of having an alias in the 64-bit address space point
into the actual 36/48/... virtual address space. I have a ppc64 ld
instruction in mind, for which a full 64-bit register would be set up
that could not fully be represented in the virtual address space.

But maybe I'm misunderstanding what exactly these functions are being
assigned for below...

Andreas

> @@ -154,6 +170,7 @@ void target_disas(FILE *out, target_ulong code, 
> target_ulong size, int flags)
>  disasm_info.read_memory_func = target_read_memory;
>  disasm_info.buffer_vma = code;
>  disasm_info.buffer_length = size;
> +disasm_info.print_address_func = generic_print_target_address;
>  
>  #ifdef TARGET_WORDS_BIGENDIAN
>  disasm_info.endian = BFD_ENDIAN_BIG;
> @@ -274,6 +291,7 @@ void disas(FILE *out, void *code, unsigned long size)
>  int (*print_insn)(bfd_vma pc, disassemble_info *info);
>  
>  INIT_DISASSEMBLE_INFO(disasm_info, out, fprintf);
> +disasm_info.print_address_func = generic_print_host_address;
>  
>  disasm_info.buffer = code;
>  disasm_info.buffer_vma = (uintptr_t)code;
> @@ -386,6 +404,7 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>  monitor_disas_env = env;
>  monitor_disas_is_physical = is_physical;
>  disasm_info.read_memory_func = monitor_read_memory;
> +disasm_info.print_address_func = generic_print_target_address;
>  
>  disasm_info.buffer_vma = pc;
>  

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





Re: [Qemu-devel] [PATCH] flatload: fix bss clearing

2012-07-09 Thread Peter Maydell
On 9 July 2012 14:04, Mike Frysinger  wrote:
> The current bss clear logic assumes the target mmap address and host
> address are the same.  Use g2h to translate from the target address
> space to the host so we can call memset on it.
>
> Signed-off-by: Mike Frysinger 

Reviewed-by: Peter Maydell 

-- PMM



[Qemu-devel] [PATCH] flatload: fix bss clearing

2012-07-09 Thread Mike Frysinger
The current bss clear logic assumes the target mmap address and host
address are the same.  Use g2h to translate from the target address
space to the host so we can call memset on it.

Signed-off-by: Mike Frysinger 
---
 linux-user/flatload.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index be79496..58f679e 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -660,7 +660,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 }
 
 /* zero the BSS.  */
-memset((void *)((unsigned long)datapos + data_len), 0, bss_len);
+memset(g2h(datapos + data_len), 0, bss_len);
 
 return 0;
 }
-- 
1.7.7.3




Re: [Qemu-devel] [PATCH] disas: Fix printing of addresses in disassembly

2012-07-09 Thread Peter Maydell
On 9 July 2012 13:45, Andreas Färber  wrote:
> Am 09.07.2012 12:27, schrieb Peter Maydell:
>> On 25 June 2012 15:55, Peter Maydell  wrote:
>>> In our disassembly code, the bfd_vma type is always 64 bits,
>>> even if the target's virtual address width is only 32 bits. This
>>> means that when we print out addresses we need to truncate them
>>> to 32 bits, to avoid odd output which has incorrectly sign-extended
>>> a value to 64 bits, for instance this ARM example:
>>> 0x80479a60:  e59f4088 ldr  r4, [pc, #136]  ; 0x80479a4f
>>>
>>> (It would also be possible to truncate before passing the address
>>> to info->print_address_func(), but truncating in the final print
>>> function is the same approach that binutils takes to this problem.)
>
> Is this bug fixed in binutils and didn't make it into QEMU due to GPLv3?
> Or is this in QEMU glue code?

In binutils it is handled in the specific implementation of the
"print a target address" callback function in objdump. That's
not part of the code we have borrowed from binutils; the functions
changed by this patch are all QEMU glue code AFAIK.

-- PMM



Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options

2012-07-09 Thread Markus Armbruster
Eric Blake  writes:

> On 07/09/2012 06:10 AM, Peter Maydell wrote:
>> On 9 July 2012 13:07, Eric Blake  wrote:
>>> That is, we are filtering based on the explicit presence of a literal
>>> '?' in the help output to determine whether we can further filter based
>>> on '-device device,?' queries without confusing qemu or libvirt;
>>> changing the 'help' output means that old libvirt with new qemu won't
>>> run the extra queries (as if it had been targeting qemu 0.12.x), even
>>> though the older spelling of the query would still work.
>>>
>>> If that is not a concern (that is, if you are willing to state that use
>>> of newer qemu is intended to be coupled with newer libvirt that has been
>>> taught to use 'help' instead of '?'), then this patch is probably fine.
>> 
>> My personal position would be that people who parse -help
>> output deserve to get bitten, but I don't get to make that
>> call :-)
>
> Obviously, newer libvirt can just proceed to try '-device device,help'
> rather than attempting to parse -h output in the first place (that is,
> making this change will not cause undue grief to libvirt 0.10.0, because
> it's a very short patch to teach libvirt.git to parse this new scheme,
> even without relying on -h output).  That is, my concern is not about
> what future libvirt will be fixed to do, but about the existing
> interaction of old libvirt with qemu (libvirt 0.9.13 + qemu 1.2 would
> fail to take full advantage of the qemu 1.2 features because of the
> change in the -h output that libvirt 0.9.13 was hard-coded to expect).
>
> And yes, I agree that libvirt has been bitten more than once because it
> tries to parse unstable -h output, but that is only because qemu has
> never provided anything more stable and more machine-parseable than -h
> output, even though the topic has come up several times in the past.
> For example, we still don't have a way to run 'query-commands' to see
> what the QMP monitor will support without first creating a dummy guest.

Yes, several attempts to provide something real & useful have been shot
down in favor of something hypothetical & perfect.

Thus, libvirt has no choice but working with the (poor) tools we give
it: version number, try and see whether it fails, help output.

Try and see can be slow, and there have been cases where you can't
easily see whether it fails.

Version number breaks down for backports.

Help breaks down when the help text changes.

Perhaps the following could reduce breakage.  Say the feature to be
tested appeared in fully usable form in upstream version X.  If the
version number is at least X, assume we have the feature.  Else, if
detecting a backport of the feature is worth it, check the help output
for a feature signature.  That way, your feature test becomes immune to
future help changes.  Downside, it becomes vulnerable to features
vanishing in the future.  But QEMU shouldn't do that to you without
sufficient prior notice.



Re: [Qemu-devel] [PATCH 3/5] target-i386: call x86_cpu_realize() after APIC is initialized.

2012-07-09 Thread Andreas Färber
Am 09.07.2012 12:59, schrieb igor:
> On 06/20/2012 03:35 PM, Andreas Färber wrote:
>> Am 20.06.2012 14:59, schrieb Igor Mammedov:
>>> It's not correct to make CPU runnable (i.e. calling x86_cpu_realize())
>>> when not all properties are set (APIC in this case).
>>>
>>> Fix it by calling x86_cpu_realize() at board level after APIC is
>>> initialized, right before cpu_reset().
>>>
>>> Signed-off-by: Igor Mammedov 
>>> ---
>>>   hw/pc.c  |1 +
>>>   target-i386/helper.c |2 --
>>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 8368701..8a662cf 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -948,6 +948,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
>>>   env->apic_state = apic_init(env, env->cpuid_apic_id);
>>>   }
>>>   qemu_register_reset(pc_cpu_reset, cpu);
>>> +x86_cpu_realize(OBJECT(cpu), NULL);
>>>   pc_cpu_reset(cpu);
>>>   return cpu;
>>>   }
>>> diff --git a/target-i386/helper.c b/target-i386/helper.c
>>> index c52ec13..b38ea7f 100644
>>> --- a/target-i386/helper.c
>>> +++ b/target-i386/helper.c
>>> @@ -1161,8 +1161,6 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>>>   return NULL;
>>>   }
>>>
>>> -x86_cpu_realize(OBJECT(cpu), NULL);
>>> -
>>>   return cpu;
>>>   }
>>>
>>
>> This will require changes in linux-user and possibly bsd-user. Having a
>> cpu_realize() would probably help with avoiding #ifdef'ery.
>> Unfortunately deriving CPUState from DeviceState proves a bit difficult
>> in the meantime (it worked at one point, now there's lots of circular
>> header dependencies), and realize support for Object got stopped.
>>
> As alternative to keep, I could leave x86_cpu_realize() in
> cpu_x86_init() and keep pc_cpu_reset() in pc_new_cpu(). That will result
> in calling cpu_reset() 3 instead of 2 times.
> Later when apic_init is moved inside cpu.c, a pc_cpu_reset() in
> pc_new_cpu() would be unnecessary and could be cleaned up then.

Let me explain in more detail what I was thinking about: cpu_init() and
cpu_x86_init() today return an initialized/realized object. I don't want
bugs to creep into the user emulators because someone is not aware that
x86 is semantically differing from all other targets.

What I did for a qemu-rl78 machine is to inline cpu_rl78_init() so that
I could put code in between, i.e., for x86: object_new(), APIC/BSP
stuff, x86_cpu_realize(). That way any addition to the realize function
will still affect the user emulators.
The downside is that when we add x86 CPU subclasses we'd have to
remember to update two places. The solution to that would be to split
out a x86_cpu_new() function used from cpu_x86_init() and wherever you
need it for the PC machine. Then the code is still maintainable in one
central place and you get to do your APIC cleanups, and we don't depend
on a central realize implementation or device parent, what do you think?

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





[Qemu-devel] [Bug 989225] Re: windows XP Standard PC HAL does not work with qemu-kvm >= 0.15

2012-07-09 Thread Michael Chudobiak
This seems to be fixed in Fedora rawhide packages. Works for me now
with:

This issue seems to be fixed in current rawhide packages:

qemu-common.x86_64 2:1.1.0-4.fc18 
qemu-img.x86_64 2:1.1.0-4.fc18
qemu-kvm.x86_64 2:1.1.0-4.fc18
qemu-system-x86.x86_64 2:1.1.0-4.fc18

Closing bug.

- Mike

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

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

Title:
  windows XP Standard PC HAL does not work with qemu-kvm >= 0.15

Status in QEMU:
  Fix Released

Bug description:
  Basically, this is a clone of a Debian bug which is explained well
  here:

  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=647312

  For some reason, the Debian bug has been archived, but I do not think
  the issue has been fixed. Basically, mouse and network do not work for
  older XP installs with the Standard PC HAL. New XP installs are fine.
  Details in the Debian bug.

  I am experiencing the same problems on a Fedora 16 64-bit host, with
  older 32-bit Windows XP guests that worked fine in Fedora 15. The
  tablet mouse and the network do not work, as described in the Debian
  bug. This used to work in Fedora 15:

  /usr/bin/qemu-kvm -hda /var/lib/libvirt/images/MikeWindows.img
  -usbdevice tablet -m 2048 -k en-us -vga qxl -spice port=5930,disable-
  ticketing

  It does not work in F16, with:
  [root@xena ~]# rpm -qa | grep qemu
  qemu-common-0.15.1-4.fc16.x86_64
  qemu-kvm-0.15.1-4.fc16.x86_64
  qemu-system-x86-0.15.1-4.fc16.x86_64
  qemu-img-0.15.1-4.fc16.x86_64
  gpxe-roms-qemu-1.0.1-4.fc16.noarch

  - Mike

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



Re: [Qemu-devel] [PATCH] disas: Fix printing of addresses in disassembly

2012-07-09 Thread Andreas Färber
Am 09.07.2012 12:27, schrieb Peter Maydell:
> Ping? [patchwork url http://patchwork.ozlabs.org/patch/167132/]
> 
> -- PMM
> 
> On 25 June 2012 15:55, Peter Maydell  wrote:
>> In our disassembly code, the bfd_vma type is always 64 bits,
>> even if the target's virtual address width is only 32 bits. This
>> means that when we print out addresses we need to truncate them
>> to 32 bits, to avoid odd output which has incorrectly sign-extended
>> a value to 64 bits, for instance this ARM example:
>> 0x80479a60:  e59f4088 ldr  r4, [pc, #136]  ; 0x80479a4f
>>
>> (It would also be possible to truncate before passing the address
>> to info->print_address_func(), but truncating in the final print
>> function is the same approach that binutils takes to this problem.)

Is this bug fixed in binutils and didn't make it into QEMU due to GPLv3?
Or is this in QEMU glue code?

Andreas

>>
>> Signed-off-by: Peter Maydell 
>> ---
>>  disas.c |   19 +++
>>  1 files changed, 19 insertions(+), 0 deletions(-)
>>
>> diff --git a/disas.c b/disas.c
>> index 93d8d30..7b2acc9 100644
>> --- a/disas.c
>> +++ b/disas.c
>> @@ -64,6 +64,22 @@ generic_print_address (bfd_vma addr, struct 
>> disassemble_info *info)
>>  (*info->fprintf_func) (info->stream, "0x%" PRIx64, addr);
>>  }
>>
>> +/* Print address in hex, truncated to the width of a target virtual 
>> address. */
>> +static void
>> +generic_print_target_address(bfd_vma addr, struct disassemble_info *info)
>> +{
>> +uint64_t mask = ~0ULL >> (64 - TARGET_VIRT_ADDR_SPACE_BITS);
>> +generic_print_address(addr & mask, info);
>> +}
>> +
>> +/* Print address in hex, truncated to the width of a host virtual address. 
>> */
>> +static void
>> +generic_print_host_address(bfd_vma addr, struct disassemble_info *info)
>> +{
>> +uint64_t mask = ~0ULL >> (64 - (sizeof(void *) * 8));
>> +generic_print_address(addr & mask, info);
>> +}
>> +
>>  /* Just return the given address.  */
>>
>>  int
>> @@ -154,6 +170,7 @@ void target_disas(FILE *out, target_ulong code, 
>> target_ulong size, int flags)
>>  disasm_info.read_memory_func = target_read_memory;
>>  disasm_info.buffer_vma = code;
>>  disasm_info.buffer_length = size;
>> +disasm_info.print_address_func = generic_print_target_address;
>>
>>  #ifdef TARGET_WORDS_BIGENDIAN
>>  disasm_info.endian = BFD_ENDIAN_BIG;
>> @@ -274,6 +291,7 @@ void disas(FILE *out, void *code, unsigned long size)
>>  int (*print_insn)(bfd_vma pc, disassemble_info *info);
>>
>>  INIT_DISASSEMBLE_INFO(disasm_info, out, fprintf);
>> +disasm_info.print_address_func = generic_print_host_address;
>>
>>  disasm_info.buffer = code;
>>  disasm_info.buffer_vma = (uintptr_t)code;
>> @@ -386,6 +404,7 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>>  monitor_disas_env = env;
>>  monitor_disas_is_physical = is_physical;
>>  disasm_info.read_memory_func = monitor_read_memory;
>> +disasm_info.print_address_func = generic_print_target_address;
>>
>>  disasm_info.buffer_vma = pc;
>>
>> --
>> 1.7.1
>>
>>
> 


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





[Qemu-devel] [Bug 1022331] Re: -cpu ? causes confusion when directory has 1-character length filenames

2012-07-09 Thread Peter Maydell
Proposed patch that would allow '-cpu help' &co:
http://patchwork.ozlabs.org/patch/169798/

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

Title:
  -cpu ? causes confusion when directory has 1-character length
  filenames

Status in QEMU:
  Invalid

Bug description:
  
  When user is in a directory with 1-character long filenames, parameter -cpu ? 
causes shell to expand ? into filename, which can cause a very confused user.

  One solution would be to replace/add alias to -cpu ?, for example
  -cpulist

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



Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options

2012-07-09 Thread Daniel P. Berrange
On Mon, Jul 09, 2012 at 06:07:48AM -0600, Eric Blake wrote:
> On 07/09/2012 05:52 AM, Peter Maydell wrote:
> > For command line options which permit '?' meaning 'please list the
> > permitted values', add support for 'help' as a synonym, by abstracting
> > the check out into a helper function.
> > 
> > Update the documentation to use 'help' rather than '?', since '?'
> > is a shell metacharacter and thus prone to fail confusingly if there
> > is a single character filename in the current working directory and
> > the '?' has not been escaped. It's therefore better to steer users
> > towards 'help', though '?' is retained for backwards compatibility.
> 
> I like the idea, but this may cause grief for libvirt, which basically does:
> 
> const char *help; // = output of 'qemu -h'
> if (strstr(help, "-device driver,?")) {
> /* Cram together all device-related queries into one invocation;
>  * the output format makes it possible to distinguish what we
>  * need.  With qemu 0.13.0 and later, unrecognized '-device
>  * bogus,?' cause an error in isolation, but are silently ignored
>  * in combination with '-device ?'.  Upstream qemu 0.12.x doesn't
>  * understand '-device name,?', and always exits with status 1 for
>  * the simpler '-device ?', so this function is really only useful
>  * if -help includes "device driver,?".  */
> cmd = virCommandNewArgList(qemu,
>"-device", "?",
>"-device", "pci-assign,?",
>"-device", "virtio-blk-pci,?",
>"-device", "virtio-net-pci,?",
>"-device", "scsi-disk,?",
>NULL);
> }
> 
> That is, we are filtering based on the explicit presence of a literal
> '?' in the help output to determine whether we can further filter based
> on '-device device,?' queries without confusing qemu or libvirt;
> changing the 'help' output means that old libvirt with new qemu won't
> run the extra queries (as if it had been targeting qemu 0.12.x), even
> though the older spelling of the query would still work.
> 
> If that is not a concern (that is, if you are willing to state that use
> of newer qemu is intended to be coupled with newer libvirt that has been
> taught to use 'help' instead of '?'), then this patch is probably fine.
>  The alternative is to update 'qemu -h' output to mention both forms,
> rather than completely eradicating the ? form from documentation.


Regardless of what QEMU does here, I think we should change libvirt's
handling of this arg. I think we can probably just skip that check, and
run 'qemu -device XXX,?' unconditionally and then handle any error that
we get back from old QEMU.

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



Re: [Qemu-devel] [PATCH 0/2] usb/scsi: usb attached scsi emulation

2012-07-09 Thread Gerd Hoffmann
On 07/09/12 12:45, Paolo Bonzini wrote:
> Il 09/07/2012 12:39, Gerd Hoffmann ha scritto:
>> On 07/09/12 12:33, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>> v2 of the usb attached scsi emulation patches.  Patch #1 is almost
>>> unmodified compared to v1.  Patch #2 is new and makes UAS emulation
>>> use the new free_request callback (patch just posted by paolo) and
>>> obviously depends on that patch to compile.
>>
>> Pushed with paolos patch and all pending usb bugfixes to
>> "git://git.kraxel.org/qemu usb.56" for those who want to play with it.
> 
> Feel free to include my patch in a pull request, of course.

Ok, will do when I send out the uas pull request, just waiting a bit so
people have some time to review, also anthony is on vacation anyway atm.

cheers,
  Gerd




Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options

2012-07-09 Thread Eric Blake
On 07/09/2012 06:10 AM, Peter Maydell wrote:
> On 9 July 2012 13:07, Eric Blake  wrote:
>> That is, we are filtering based on the explicit presence of a literal
>> '?' in the help output to determine whether we can further filter based
>> on '-device device,?' queries without confusing qemu or libvirt;
>> changing the 'help' output means that old libvirt with new qemu won't
>> run the extra queries (as if it had been targeting qemu 0.12.x), even
>> though the older spelling of the query would still work.
>>
>> If that is not a concern (that is, if you are willing to state that use
>> of newer qemu is intended to be coupled with newer libvirt that has been
>> taught to use 'help' instead of '?'), then this patch is probably fine.
> 
> My personal position would be that people who parse -help
> output deserve to get bitten, but I don't get to make that
> call :-)

Obviously, newer libvirt can just proceed to try '-device device,help'
rather than attempting to parse -h output in the first place (that is,
making this change will not cause undue grief to libvirt 0.10.0, because
it's a very short patch to teach libvirt.git to parse this new scheme,
even without relying on -h output).  That is, my concern is not about
what future libvirt will be fixed to do, but about the existing
interaction of old libvirt with qemu (libvirt 0.9.13 + qemu 1.2 would
fail to take full advantage of the qemu 1.2 features because of the
change in the -h output that libvirt 0.9.13 was hard-coded to expect).

And yes, I agree that libvirt has been bitten more than once because it
tries to parse unstable -h output, but that is only because qemu has
never provided anything more stable and more machine-parseable than -h
output, even though the topic has come up several times in the past.
For example, we still don't have a way to run 'query-commands' to see
what the QMP monitor will support without first creating a dummy guest.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH V2] Use clean shutdown request for ctrl-a x

2012-07-09 Thread Andreas Färber
Am 09.07.2012 12:19, schrieb Fabien Chouteau:
> Any comment?
> 
> On 07/04/2012 01:04 PM, Fabien Chouteau wrote:
>> The goal is to make ctrl-a x to close Qemu in a clean way. The current
>> exit(0) skips a lot of cleanup/close functions, for example in block
>> drivers.
>>
>> Signed-off-by: Fabien Chouteau 
>> ---
>>  qemu-char.c |2 +-
>>  sysemu.h|1 +
>>  vl.c|5 +
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index c2aaaee..7732846 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -353,7 +353,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver 
>> *d, int ch)
>>  {
>>   const char *term =  "QEMU: Terminated\n\r";
>>   chr->chr_write(chr,(uint8_t *)term,strlen(term));
>> - exit(0);
>> + qemu_system_force_shutdown();
>>   break;
>>  }
>>  case 's':

FWIW there was a recent patch by Hervé that exposed further occurrences
of exit(), probably all would need to be reviewed and fixed.

Andreas

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





Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options

2012-07-09 Thread Peter Maydell
On 9 July 2012 13:07, Eric Blake  wrote:
> That is, we are filtering based on the explicit presence of a literal
> '?' in the help output to determine whether we can further filter based
> on '-device device,?' queries without confusing qemu or libvirt;
> changing the 'help' output means that old libvirt with new qemu won't
> run the extra queries (as if it had been targeting qemu 0.12.x), even
> though the older spelling of the query would still work.
>
> If that is not a concern (that is, if you are willing to state that use
> of newer qemu is intended to be coupled with newer libvirt that has been
> taught to use 'help' instead of '?'), then this patch is probably fine.

My personal position would be that people who parse -help
output deserve to get bitten, but I don't get to make that
call :-)

-- PMM



Re: [Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options

2012-07-09 Thread Eric Blake
On 07/09/2012 05:52 AM, Peter Maydell wrote:
> For command line options which permit '?' meaning 'please list the
> permitted values', add support for 'help' as a synonym, by abstracting
> the check out into a helper function.
> 
> Update the documentation to use 'help' rather than '?', since '?'
> is a shell metacharacter and thus prone to fail confusingly if there
> is a single character filename in the current working directory and
> the '?' has not been escaped. It's therefore better to steer users
> towards 'help', though '?' is retained for backwards compatibility.

I like the idea, but this may cause grief for libvirt, which basically does:

const char *help; // = output of 'qemu -h'
if (strstr(help, "-device driver,?")) {
/* Cram together all device-related queries into one invocation;
 * the output format makes it possible to distinguish what we
 * need.  With qemu 0.13.0 and later, unrecognized '-device
 * bogus,?' cause an error in isolation, but are silently ignored
 * in combination with '-device ?'.  Upstream qemu 0.12.x doesn't
 * understand '-device name,?', and always exits with status 1 for
 * the simpler '-device ?', so this function is really only useful
 * if -help includes "device driver,?".  */
cmd = virCommandNewArgList(qemu,
   "-device", "?",
   "-device", "pci-assign,?",
   "-device", "virtio-blk-pci,?",
   "-device", "virtio-net-pci,?",
   "-device", "scsi-disk,?",
   NULL);
}

That is, we are filtering based on the explicit presence of a literal
'?' in the help output to determine whether we can further filter based
on '-device device,?' queries without confusing qemu or libvirt;
changing the 'help' output means that old libvirt with new qemu won't
run the extra queries (as if it had been targeting qemu 0.12.x), even
though the older spelling of the query would still work.

If that is not a concern (that is, if you are willing to state that use
of newer qemu is intended to be coupled with newer libvirt that has been
taught to use 'help' instead of '?'), then this patch is probably fine.
 The alternative is to update 'qemu -h' output to mention both forms,
rather than completely eradicating the ? form from documentation.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2 1/3] KVM: Add new -cpu best

2012-07-09 Thread Alexander Graf

On 02.07.2012, at 16:25, Avi Kivity wrote:

> On 06/26/2012 07:39 PM, Alexander Graf wrote:
>> During discussions on whether to make -cpu host the default in SLE, I found
>> myself disagreeing to the thought, because it potentially opens a big can
>> of worms for potential bugs. But if I already am so opposed to it for SLE, 
>> how
>> can it possibly be reasonable to default to -cpu host in upstream QEMU? And
>> what would a sane default look like?
>> 
>> So I had this idea of looping through all available CPU definitions. We can
>> pretty well tell if our host is able to execute any of them by checking the
>> respective flags and seeing if our host has all features the CPU definition
>> requires. With that, we can create a -cpu type that would fall back to the
>> "best known CPU definition" that our host can fulfill. On my Phenom II
>> system for example, that would be -cpu phenom.
>> 
>> With this approach we can test and verify that CPU types actually work at
>> any random user setup, because we can always verify that all the -cpu types
>> we ship actually work. And we only default to some clever mechanism that
>> chooses from one of these.
>> 
>> 
>> +/* Are all guest feature bits present on the host? */
>> +static bool cpu_x86_feature_subset(uint32_t host, uint32_t guest)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < 32; i++) {
>> +uint32_t mask = 1 << i;
>> +if ((guest & mask) && !(host & mask)) {
>> +return false;
>> +}
>> +}
>> +
>> +return true;
> 
>return !(guest & ~host);

I guess it helps to think :).

> 
> 
>> +}
> 
> 
> 
>> +
>> +
>> +
>> +static void cpu_x86_fill_best(x86_def_t *x86_cpu_def)
>> +{
>> +x86_def_t *def;
>> +
>> +x86_cpu_def->family = 0;
>> +x86_cpu_def->model = 0;
>> +for (def = x86_defs; def; def = def->next) {
>> +if (cpu_x86_fits_host(def) && cpu_x86_fits_higher(def, 
>> x86_cpu_def)) {
>> +memcpy(x86_cpu_def, def, sizeof(*def));
>> +}
>  *x86_cpu_def = *def;
>> +}
>> +
>> +if (!x86_cpu_def->family && !x86_cpu_def->model) {
>> +fprintf(stderr, "No fitting CPU model found!\n");
>> +exit(1);
>> +}
>> +}
>> +
>> static int unavailable_host_feature(struct model_features_t *f, uint32_t 
>> mask)
>> {
>> int i;
>> @@ -878,6 +957,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
>> const char *cpu_model)
>> break;
>> if (kvm_enabled() && name && strcmp(name, "host") == 0) {
>> cpu_x86_fill_host(x86_cpu_def);
>> +} else if (kvm_enabled() && name && strcmp(name, "best") == 0) {
>> +cpu_x86_fill_best(x86_cpu_def);
>> } else if (!def) {
>> goto error;
>> } else {
>> 
> 
> Should we copy the cache size etc. from the host?

I don't think so. We should rather make sure we always have cpu descriptions 
available close to what people out there actually use.


Alex




[Qemu-devel] [PATCH] Support 'help' as a synonym for '?' in command line options

2012-07-09 Thread Peter Maydell
For command line options which permit '?' meaning 'please list the
permitted values', add support for 'help' as a synonym, by abstracting
the check out into a helper function.

Update the documentation to use 'help' rather than '?', since '?'
is a shell metacharacter and thus prone to fail confusingly if there
is a single character filename in the current working directory and
the '?' has not been escaped. It's therefore better to steer users
towards 'help', though '?' is retained for backwards compatibility.

Signed-off-by: Peter Maydell 
---
Patch is based on grepping the source for '?' and "?"; I think I've
caught everything that needed changing but it's possible I missed
something.

NB: the bsd-user patch isn't compile tested since I don't have a BSD box.

 arch_init.c   |4 ++--
 blockdev.c|   10 +-
 bsd-user/main.c   |6 +++---
 hw/mips_jazz.c|2 +-
 hw/qdev-monitor.c |2 +-
 hw/watchdog.c |2 +-
 linux-user/main.c |4 ++--
 net.c |3 ++-
 qemu-common.h |   16 
 qemu-doc.texi |2 +-
 qemu-ga.c |2 +-
 qemu-img.c|4 ++--
 qemu-options.hx   |   32 
 qemu-timer.c  |2 +-
 vl.c  |4 ++--
 15 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index a9e8b74..e8e6f80 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -602,7 +602,7 @@ void select_soundhw(const char *optarg)
 {
 struct soundhw *c;
 
-if (*optarg == '?') {
+if (is_help_option(optarg)) {
 show_valid_cards:
 
 printf("Valid sound card names (comma separated):\n");
@@ -610,7 +610,7 @@ void select_soundhw(const char *optarg)
 printf ("%-11s %s\n", c->name, c->descr);
 }
 printf("\n-soundhw all will enable all of the above\n");
-exit(*optarg != '?');
+exit(!is_help_option(optarg));
 }
 else {
 size_t l;
diff --git a/blockdev.c b/blockdev.c
index 9e0a72a..7810cd7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -398,11 +398,11 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 #endif
 
 if ((buf = qemu_opt_get(opts, "format")) != NULL) {
-   if (strcmp(buf, "?") == 0) {
-   error_printf("Supported formats:");
-   bdrv_iterate_format(bdrv_format_print, NULL);
-   error_printf("\n");
-   return NULL;
+if (is_help_option(buf)) {
+error_printf("Supported formats:");
+bdrv_iterate_format(bdrv_format_print, NULL);
+error_printf("\n");
+return NULL;
 }
 drv = bdrv_find_whitelisted_format(buf);
 if (!drv) {
diff --git a/bsd-user/main.c b/bsd-user/main.c
index cd33d65..0aad752 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -681,7 +681,7 @@ static void usage(void)
"-g port   wait gdb connection to port\n"
"-L path   set the elf interpreter prefix (default=%s)\n"
"-s size   set the stack size in bytes (default=%ld)\n"
-   "-cpu modelselect CPU (-cpu ? for list)\n"
+   "-cpu modelselect CPU (-cpu help for list)\n"
"-drop-ld-preload  drop LD_PRELOAD for target process\n"
"-E var=value  sets/modifies targets environment variable(s)\n"
"-U varunsets targets environment variable(s)\n"
@@ -825,8 +825,8 @@ int main(int argc, char **argv)
 qemu_uname_release = argv[optind++];
 } else if (!strcmp(r, "cpu")) {
 cpu_model = argv[optind++];
-if (strcmp(cpu_model, "?") == 0) {
-/* XXX: implement xxx_cpu_list for targets that still miss it */
+if (is_help_option(cpu_model)) {
+/* XXX: implement xxx_cpu_list for targets that still miss it 
*/
 #if defined(cpu_list)
 cpu_list(stdout, &fprintf);
 #endif
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index bf1b799..db927f1 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -239,7 +239,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
 dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
  rc4030_opaque, rc4030_dma_memory_rw);
 break;
-} else if (strcmp(nd->model, "?") == 0) {
+} else if (is_help_option(nd->model)) {
 fprintf(stderr, "qemu: Supported NICs: dp83932\n");
 exit(1);
 } else {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 7915b45..d777a20 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -138,7 +138,7 @@ int qdev_device_help(QemuOpts *opts)
 ObjectClass *klass;
 
 driver = qemu_opt_get(opts, "driver");
-if (driver && !strcmp(driver, "?")) {
+if (driver && is_help_option(driver)) {
 bool show_no_user = false;
 object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, 
&show_no_user);
 return 1;
diff --gi

[Qemu-devel] [PATCH 9/9] usb-host: add trace events for iso xfers

2012-07-09 Thread Gerd Hoffmann
Replace iso transfer fprintf's with trace points.  Also rename existing
tracepoints so they all match usb_host_iso_*.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/host-linux.c |   10 ++
 trace-events|6 --
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 9ba8925..d55be87 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -213,7 +213,7 @@ static int is_iso_started(USBHostDevice *s, int pid, int ep)
 
 static void clear_iso_started(USBHostDevice *s, int pid, int ep)
 {
-trace_usb_host_ep_stop_iso(s->bus_num, s->addr, ep);
+trace_usb_host_iso_stop(s->bus_num, s->addr, ep);
 get_endp(s, pid, ep)->iso_started = 0;
 }
 
@@ -221,7 +221,7 @@ static void set_iso_started(USBHostDevice *s, int pid, int 
ep)
 {
 struct endp_data *e = get_endp(s, pid, ep);
 
-trace_usb_host_ep_start_iso(s->bus_num, s->addr, ep);
+trace_usb_host_iso_start(s->bus_num, s->addr, ep);
 if (!e->iso_started) {
 e->iso_started = 1;
 e->inflight = 0;
@@ -319,7 +319,8 @@ static void async_complete(void *opaque)
 if (r < 0) {
 if (errno == EAGAIN) {
 if (urbs > 2) {
-fprintf(stderr, "husb: %d iso urbs finished at once\n", 
urbs);
+/* indicates possible latency issues */
+trace_usb_host_iso_many_urbs(s->bus_num, s->addr, urbs);
 }
 return;
 }
@@ -352,7 +353,8 @@ static void async_complete(void *opaque)
 urbs++;
 inflight = change_iso_inflight(s, pid, ep, -1);
 if (inflight == 0 && is_iso_started(s, pid, ep)) {
-fprintf(stderr, "husb: out of buffers for iso stream\n");
+/* can be latency issues, or simply end of stream */
+trace_usb_host_iso_out_of_bufs(s->bus_num, s->addr, ep);
 }
 continue;
 }
diff --git a/trace-events b/trace-events
index c935ba2..c33d58c 100644
--- a/trace-events
+++ b/trace-events
@@ -368,8 +368,10 @@ usb_host_urb_complete(int bus, int addr, void *aurb, int 
status, int length, int
 usb_host_urb_canceled(int bus, int addr, void *aurb) "dev %d:%d, aurb %p"
 usb_host_ep_set_halt(int bus, int addr, int ep) "dev %d:%d, ep %d"
 usb_host_ep_clear_halt(int bus, int addr, int ep) "dev %d:%d, ep %d"
-usb_host_ep_start_iso(int bus, int addr, int ep) "dev %d:%d, ep %d"
-usb_host_ep_stop_iso(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_start(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_stop(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_out_of_bufs(int bus, int addr, int ep) "dev %d:%d, ep %d"
+usb_host_iso_many_urbs(int bus, int addr, int count) "dev %d:%d, count %d"
 usb_host_reset(int bus, int addr) "dev %d:%d"
 usb_host_auto_scan_enabled(void)
 usb_host_auto_scan_disabled(void)
-- 
1.7.1




[Qemu-devel] [PULL 0/9] usb patch queue

2012-07-09 Thread Gerd Hoffmann
  Hi,

Here comes the most recent usb patch queue, featuring a collection of
little bug fixes all over the place.  See individual patches for
details.

please pull,
  Gerd

The following changes since commit 84988cf910a6881f2180fdcec516b60f8f0dc8c4:

  bitops.h: Add functions to extract and deposit bitfields (2012-07-07 09:07:01 
+)

are available in the git repository at:
  git://git.kraxel.org/qemu usb.55

Gerd Hoffmann (6):
  ehci: fix ehci_qh_do_overlay
  ehci: fix td writeback
  ehci: don't flush cache on doorbell rings.
  usb: split endpoint init and reset
  usb: fix interface initialization
  usb-host: add trace events for iso xfers

Hans de Goede (3):
  usb-ehci: Fix an assert whenever isoc transfers are used
  ehci: Kick async schedule on wakeup in the non companion case
  usb-redir: Correctly handle the usb_redir_babble usbredir status

 hw/usb.h|3 ++
 hw/usb/core.c   |   17 --
 hw/usb/hcd-ehci.c   |   84 +-
 hw/usb/host-linux.c |   15 +
 hw/usb/redirect.c   |2 +
 trace-events|6 ++-
 6 files changed, 86 insertions(+), 41 deletions(-)



Re: [Qemu-devel] [PATCH V2] Use clean shutdown request for ctrl-a x

2012-07-09 Thread Paolo Bonzini
Il 09/07/2012 12:19, Fabien Chouteau ha scritto:
> Any comment?

Looks good to me.  Blue, can you apply it while Anthony is on holiday?

Paolo

> On 07/04/2012 01:04 PM, Fabien Chouteau wrote:
>> The goal is to make ctrl-a x to close Qemu in a clean way. The current
>> exit(0) skips a lot of cleanup/close functions, for example in block
>> drivers.
>>
>> Signed-off-by: Fabien Chouteau 
>> ---
>>  qemu-char.c |2 +-
>>  sysemu.h|1 +
>>  vl.c|5 +
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index c2aaaee..7732846 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -353,7 +353,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver 
>> *d, int ch)
>>  {
>>   const char *term =  "QEMU: Terminated\n\r";
>>   chr->chr_write(chr,(uint8_t *)term,strlen(term));
>> - exit(0);
>> + qemu_system_force_shutdown();
>>   break;
>>  }
>>  case 's':
>> diff --git a/sysemu.h b/sysemu.h
>> index bc2c788..2b2c354 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -50,6 +50,7 @@ void qemu_register_suspend_notifier(Notifier *notifier);
>>  void qemu_system_wakeup_request(WakeupReason reason);
>>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>>  void qemu_register_wakeup_notifier(Notifier *notifier);
>> +void qemu_system_force_shutdown(void);
>>  void qemu_system_shutdown_request(void);
>>  void qemu_system_powerdown_request(void);
>>  void qemu_system_debug_request(void);
>> diff --git a/vl.c b/vl.c
>> index 1329c30..bd27583 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1477,6 +1477,11 @@ void qemu_system_killed(int signal, pid_t pid)
>>  {
>>  shutdown_signal = signal;
>>  shutdown_pid = pid;
>> +qemu_system_force_shutdown();
>> +}
>> +
>> +void qemu_system_force_shutdown(void)
>> +{
>>  no_shutdown = 0;
>>  qemu_system_shutdown_request();
>>  }
>>
> 
> 





[Qemu-devel] [PATCH 1/9] ehci: fix ehci_qh_do_overlay

2012-07-09 Thread Gerd Hoffmann
Use ehci_flush_qh to make sure we touch inly the fields the hc is
allowed to touch.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ehci.c |   37 ++---
 1 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 1582c2c..7de47e5 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1246,6 +1246,23 @@ static inline int put_dwords(EHCIState *ehci, uint32_t 
addr,
 return 1;
 }
 
+/*
+ *  Write the qh back to guest physical memory.  This step isn't
+ *  in the EHCI spec but we need to do it since we don't share
+ *  physical memory with our guest VM.
+ *
+ *  The first three dwords are read-only for the EHCI, so skip them
+ *  when writing back the qh.
+ */
+static void ehci_flush_qh(EHCIQueue *q)
+{
+uint32_t *qh = (uint32_t *) &q->qh;
+uint32_t dwords = sizeof(EHCIqh) >> 2;
+uint32_t addr = NLPTR_GET(q->qhaddr);
+
+put_dwords(q->ehci, addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
+}
+
 // 4.10.2
 
 static int ehci_qh_do_overlay(EHCIQueue *q)
@@ -1293,8 +1310,7 @@ static int ehci_qh_do_overlay(EHCIQueue *q)
 q->qh.bufptr[1] &= ~BUFPTR_CPROGMASK_MASK;
 q->qh.bufptr[2] &= ~BUFPTR_FRAMETAG_MASK;
 
-put_dwords(q->ehci, NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh,
-   sizeof(EHCIqh) >> 2);
+ehci_flush_qh(q);
 
 return 0;
 }
@@ -1600,23 +1616,6 @@ static int ehci_process_itd(EHCIState *ehci,
 }
 
 
-/*
- *  Write the qh back to guest physical memory.  This step isn't
- *  in the EHCI spec but we need to do it since we don't share
- *  physical memory with our guest VM.
- *
- *  The first three dwords are read-only for the EHCI, so skip them
- *  when writing back the qh.
- */
-static void ehci_flush_qh(EHCIQueue *q)
-{
-uint32_t *qh = (uint32_t *) &q->qh;
-uint32_t dwords = sizeof(EHCIqh) >> 2;
-uint32_t addr = NLPTR_GET(q->qhaddr);
-
-put_dwords(q->ehci, addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
-}
-
 /*  This state is the entry point for asynchronous schedule
  *  processing.  Entry here consitutes a EHCI start event state (4.8.5)
  */
-- 
1.7.1




[Qemu-devel] [PATCH v3 03/10] esp: implement Disable selection command

2012-07-09 Thread Hervé Poussineau

Signed-off-by: Hervé Poussineau 
---
 hw/esp.c |6 ++
 trace-events |1 +
 2 files changed, 7 insertions(+)

diff --git a/hw/esp.c b/hw/esp.c
index ac91f00..985a2ee 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -117,6 +117,7 @@ struct ESPState {
 #define CMD_SELATN   0x42
 #define CMD_SELATNS  0x43
 #define CMD_ENSEL0x44
+#define CMD_DISSEL   0x45
 
 #define STAT_DO 0x00
 #define STAT_DI 0x01
@@ -649,6 +650,11 @@ static void esp_mem_write(void *opaque, target_phys_addr_t 
addr,
 trace_esp_mem_writeb_cmd_ensel(val);
 s->rregs[ESP_RINTR] = 0;
 break;
+case CMD_DISSEL:
+trace_esp_mem_writeb_cmd_dissel(val);
+s->rregs[ESP_RINTR] = 0;
+esp_raise_irq(s);
+break;
 default:
 ESP_ERROR("Unhandled ESP command (%2.2x)\n", (unsigned)val);
 break;
diff --git a/trace-events b/trace-events
index c935ba2..ba14f75 100644
--- a/trace-events
+++ b/trace-events
@@ -674,6 +674,7 @@ esp_mem_writeb_cmd_sel(uint32_t val) "Select without ATN 
(%2.2x)"
 esp_mem_writeb_cmd_selatn(uint32_t val) "Select with ATN (%2.2x)"
 esp_mem_writeb_cmd_selatns(uint32_t val) "Select with ATN & stop (%2.2x)"
 esp_mem_writeb_cmd_ensel(uint32_t val) "Enable selection (%2.2x)"
+esp_mem_writeb_cmd_dissel(uint32_t val) "Disable selection (%2.2x)"
 
 # monitor.c
 handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\""
-- 
1.7.10.4




[Qemu-devel] [PATCH 7/9] usb: split endpoint init and reset

2012-07-09 Thread Gerd Hoffmann
Create a new usb_ep_reset() function to reset endpoint state, without
re-initialiting the queues, so we don't unlink in-flight packets just
because usb-host has to re-parse the descriptor tables.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb.h|1 +
 hw/usb/core.c   |   13 +++--
 hw/usb/host-linux.c |5 +++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index a5623d3..9cd2f89 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -363,6 +363,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p);
 void usb_cancel_packet(USBPacket * p);
 
 void usb_ep_init(USBDevice *dev);
+void usb_ep_reset(USBDevice *dev);
 void usb_ep_dump(USBDevice *dev);
 struct USBEndpoint *usb_ep_get(USBDevice *dev, int pid, int ep);
 uint8_t usb_ep_get_type(USBDevice *dev, int pid, int ep);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 0e02da7..fe15be0 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -550,7 +550,7 @@ void usb_packet_cleanup(USBPacket *p)
 qemu_iovec_destroy(&p->iov);
 }
 
-void usb_ep_init(USBDevice *dev)
+void usb_ep_reset(USBDevice *dev)
 {
 int ep;
 
@@ -559,7 +559,6 @@ void usb_ep_init(USBDevice *dev)
 dev->ep_ctl.ifnum = 0;
 dev->ep_ctl.dev = dev;
 dev->ep_ctl.pipeline = false;
-QTAILQ_INIT(&dev->ep_ctl.queue);
 for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
 dev->ep_in[ep].nr = ep + 1;
 dev->ep_out[ep].nr = ep + 1;
@@ -573,6 +572,16 @@ void usb_ep_init(USBDevice *dev)
 dev->ep_out[ep].dev = dev;
 dev->ep_in[ep].pipeline = false;
 dev->ep_out[ep].pipeline = false;
+}
+}
+
+void usb_ep_init(USBDevice *dev)
+{
+int ep;
+
+usb_ep_reset(dev);
+QTAILQ_INIT(&dev->ep_ctl.queue);
+for (ep = 0; ep < USB_MAX_ENDPOINTS; ep++) {
 QTAILQ_INIT(&dev->ep_in[ep].queue);
 QTAILQ_INIT(&dev->ep_out[ep].queue);
 }
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index 5479fb5..9ba8925 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -1136,7 +1136,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
 USBDescriptor *d;
 bool active = false;
 
-usb_ep_init(&s->dev);
+usb_ep_reset(&s->dev);
 
 for (i = 0;; i += d->bLength) {
 if (i+2 >= s->descr_len) {
@@ -1239,7 +1239,7 @@ static int usb_linux_update_endp_table(USBHostDevice *s)
 return 0;
 
 error:
-usb_ep_init(&s->dev);
+usb_ep_reset(&s->dev);
 return 1;
 }
 
@@ -1326,6 +1326,7 @@ static int usb_host_open(USBHostDevice *dev, int bus_num,
 goto fail;
 }
 
+usb_ep_init(&dev->dev);
 ret = usb_linux_update_endp_table(dev);
 if (ret) {
 goto fail;
-- 
1.7.1




Re: [Qemu-devel] [PATCH v2] ahci: add -drive support

2012-07-09 Thread Alexander Graf

On 09.07.2012, at 13:19, Markus Armbruster wrote:

> Alexander Graf  writes:
> 
>> On 09.07.2012, at 13:06, Markus Armbruster wrote:
>> 
>>> Kevin Wolf  writes:
>>> 
 Am 09.07.2012 10:50, schrieb Markus Armbruster:
> Alexander Graf  writes:
> 
>> We've had support for creating AHCI devices using -device for a while 
>> now,
>> but it's cumbersome to users. We really should provide an easier way for
>> them to leverage the power of AHCI!
>> 
>> So let's introduce a new if= option to -drive, giving users the same
>> command line experience as for scsi or ide.
>> 
>> Signed-off-by: Alexander Graf 
>> 
>> ---
>> 
>> v1 -> v2:
>> 
>> - support more than a single drive per adapter
>> - support index= option
>> - treat IF_AHCI the same as IF_IDE
> 
> Inhowfar?  Not obvious to me from the patch, or the diff patch v1.
> 
>> - add is_ata() helper to match AHCI || IDE
> 
> Not addressed:
> 
> Once we switch to q35, if=ahci will become a redundant wart: to add
> drives to the board's AHCI controller, you'll have to use if=ide.
> if=ahci will create new controllers, which is generally not what you
> want.  Ugh.
 
 Can we even make it the default with q35 as long as our AHCI controller
 doesn't also expose a legacy interface for compatibility?
>>> 
>>> Err, what else could we do with if=ide with q35?
>> 
>> We could create a cmd646 and attach drives to that.
> 
> Naive use of the command line then connects your drives to some an old
> and slow add-on controller instead of the perfectly servicable and fast
> on-board controller.
> 
> Let me rephrase my question: s/could/should/

Well, what I'm saying is that we should distinguish between -hda and if=ide. 
Make -hda attach the disk to the default controller. Make -cdrom attach the 
cdrom to the default controller. Make if=ide attach the disk to an IDE 
controller. If there's none in the system, spawn one.

That way the "naive" use case of passing no -hda or -hda -hdb is guaranteed to 
be good and fast. The case where a user actually asks for an IDE device, he 
still gets what he asked for.

Alex




[Qemu-devel] [PATCH v3 05/10] esp: support future change of chip_id

2012-07-09 Thread Hervé Poussineau

Signed-off-by: Hervé Poussineau 
---
 hw/esp.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/esp.c b/hw/esp.c
index 85078e0..a1f5b8a 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -50,6 +50,7 @@ struct ESPState {
 uint8_t wregs[ESP_REGS];
 qemu_irq irq;
 uint32_t it_shift;
+uint8_t chip_id;
 int32_t ti_size;
 uint32_t ti_rptr, ti_wptr;
 uint32_t status;
@@ -475,7 +476,7 @@ static void esp_hard_reset(DeviceState *d)
 
 memset(s->rregs, 0, ESP_REGS);
 memset(s->wregs, 0, ESP_REGS);
-s->rregs[ESP_TCHI] = TCHI_FAS100A; // Indicate fas100a
+s->rregs[ESP_TCHI] = s->chip_id;
 s->ti_size = 0;
 s->ti_rptr = 0;
 s->ti_wptr = 0;
@@ -759,6 +760,7 @@ static int esp_init1(SysBusDevice *dev)
 sysbus_init_irq(dev, &s->irq);
 assert(s->it_shift != -1);
 
+s->chip_id = TCHI_FAS100A;
 memory_region_init_io(&s->iomem, &esp_mem_ops, s,
   "esp", ESP_REGS << s->it_shift);
 sysbus_init_mmio(dev, &s->iomem);
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v2] ahci: add -drive support

2012-07-09 Thread Markus Armbruster
Alexander Graf  writes:

> On 09.07.2012, at 13:06, Markus Armbruster wrote:
>
>> Kevin Wolf  writes:
>> 
>>> Am 09.07.2012 10:50, schrieb Markus Armbruster:
 Alexander Graf  writes:
 
> We've had support for creating AHCI devices using -device for a while now,
> but it's cumbersome to users. We really should provide an easier way for
> them to leverage the power of AHCI!
> 
> So let's introduce a new if= option to -drive, giving users the same
> command line experience as for scsi or ide.
> 
> Signed-off-by: Alexander Graf 
> 
> ---
> 
> v1 -> v2:
> 
>  - support more than a single drive per adapter
>  - support index= option
>  - treat IF_AHCI the same as IF_IDE
 
 Inhowfar?  Not obvious to me from the patch, or the diff patch v1.
 
>  - add is_ata() helper to match AHCI || IDE
 
 Not addressed:
 
 Once we switch to q35, if=ahci will become a redundant wart: to add
 drives to the board's AHCI controller, you'll have to use if=ide.
 if=ahci will create new controllers, which is generally not what you
 want.  Ugh.
>>> 
>>> Can we even make it the default with q35 as long as our AHCI controller
>>> doesn't also expose a legacy interface for compatibility?
>> 
>> Err, what else could we do with if=ide with q35?
>
> We could create a cmd646 and attach drives to that.

Naive use of the command line then connects your drives to some an old
and slow add-on controller instead of the perfectly servicable and fast
on-board controller.

Let me rephrase my question: s/could/should/



[Qemu-devel] [PATCH 6/9] usb-redir: Correctly handle the usb_redir_babble usbredir status

2012-07-09 Thread Gerd Hoffmann
From: Hans de Goede 

Signed-off-by: Hans de Goede 
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/redirect.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index d949f04..10b4fbb 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1033,6 +1033,8 @@ static int usbredir_handle_status(USBRedirDevice *dev,
 case usb_redir_inval:
 WARNING("got invalid param error from usb-host?\n");
 return USB_RET_NAK;
+case usb_redir_babble:
+return USB_RET_BABBLE;
 case usb_redir_ioerror:
 case usb_redir_timeout:
 default:
-- 
1.7.1




[Qemu-devel] [PATCH v3 06/10] esp: use hba_private field instead of a complex cast

2012-07-09 Thread Hervé Poussineau

Signed-off-by: Hervé Poussineau 
---
 hw/esp.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index a1f5b8a..d9dd2aa 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -186,7 +186,7 @@ static void esp_dma_enable(void *opaque, int irq, int level)
 
 static void esp_request_cancelled(SCSIRequest *req)
 {
-ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
+ESPState *s = req->hba_private;
 
 if (req == s->current_req) {
 scsi_req_unref(s->current_req);
@@ -242,7 +242,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
 trace_esp_do_busid_cmd(busid);
 lun = busid & 7;
 current_lun = scsi_device_find(&s->bus, 0, s->current_dev->id, lun);
-s->current_req = scsi_req_new(current_lun, 0, lun, buf, NULL);
+s->current_req = scsi_req_new(current_lun, 0, lun, buf, s);
 datalen = scsi_req_enqueue(s->current_req);
 s->ti_size = datalen;
 if (datalen != 0) {
@@ -396,7 +396,7 @@ static void esp_do_dma(ESPState *s)
 static void esp_command_complete(SCSIRequest *req, uint32_t status,
  size_t resid)
 {
-ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
+ESPState *s = req->hba_private;
 
 trace_esp_command_complete();
 if (s->ti_size != 0) {
@@ -420,7 +420,7 @@ static void esp_command_complete(SCSIRequest *req, uint32_t 
status,
 
 static void esp_transfer_data(SCSIRequest *req, uint32_t len)
 {
-ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
+ESPState *s = req->hba_private;
 
 trace_esp_transfer_data(s->dma_left, s->ti_size);
 s->async_len = len;
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v2] ahci: add -drive support

2012-07-09 Thread Alexander Graf

On 09.07.2012, at 13:06, Markus Armbruster wrote:

> Kevin Wolf  writes:
> 
>> Am 09.07.2012 10:50, schrieb Markus Armbruster:
>>> Alexander Graf  writes:
>>> 
 We've had support for creating AHCI devices using -device for a while now,
 but it's cumbersome to users. We really should provide an easier way for
 them to leverage the power of AHCI!
 
 So let's introduce a new if= option to -drive, giving users the same
 command line experience as for scsi or ide.
 
 Signed-off-by: Alexander Graf 
 
 ---
 
 v1 -> v2:
 
  - support more than a single drive per adapter
  - support index= option
  - treat IF_AHCI the same as IF_IDE
>>> 
>>> Inhowfar?  Not obvious to me from the patch, or the diff patch v1.
>>> 
  - add is_ata() helper to match AHCI || IDE
>>> 
>>> Not addressed:
>>> 
>>> Once we switch to q35, if=ahci will become a redundant wart: to add
>>> drives to the board's AHCI controller, you'll have to use if=ide.
>>> if=ahci will create new controllers, which is generally not what you
>>> want.  Ugh.
>> 
>> Can we even make it the default with q35 as long as our AHCI controller
>> doesn't also expose a legacy interface for compatibility?
> 
> Err, what else could we do with if=ide with q35?

We could create a cmd646 and attach drives to that.


Alex




Re: [Qemu-devel] [PATCH v2] ahci: add -drive support

2012-07-09 Thread Markus Armbruster
Kevin Wolf  writes:

> Am 09.07.2012 10:50, schrieb Markus Armbruster:
>> Alexander Graf  writes:
>> 
>>> We've had support for creating AHCI devices using -device for a while now,
>>> but it's cumbersome to users. We really should provide an easier way for
>>> them to leverage the power of AHCI!
>>>
>>> So let's introduce a new if= option to -drive, giving users the same
>>> command line experience as for scsi or ide.
>>>
>>> Signed-off-by: Alexander Graf 
>>>
>>> ---
>>>
>>> v1 -> v2:
>>>
>>>   - support more than a single drive per adapter
>>>   - support index= option
>>>   - treat IF_AHCI the same as IF_IDE
>> 
>> Inhowfar?  Not obvious to me from the patch, or the diff patch v1.
>> 
>>>   - add is_ata() helper to match AHCI || IDE
>> 
>> Not addressed:
>> 
>> Once we switch to q35, if=ahci will become a redundant wart: to add
>> drives to the board's AHCI controller, you'll have to use if=ide.
>> if=ahci will create new controllers, which is generally not what you
>> want.  Ugh.
>
> Can we even make it the default with q35 as long as our AHCI controller
> doesn't also expose a legacy interface for compatibility?

Err, what else could we do with if=ide with q35?



Re: [Qemu-devel] [PATCH v2] ahci: add -drive support

2012-07-09 Thread Markus Armbruster
Gerd Hoffmann  writes:

>   Hi,
>
>>> If -hda has the semantics of "create an IDE device", then no, we can't 
>>> change it. It doesn't however. IIRC on -M pseries -hda creates SCSI 
>>> devices. On s390 -hda creates virtio devices. So if on -M q35 -hda would 
>>> create if=ahci, I don't see how that breaks the CLI.
>> 
>> It doesn't per se, that is as long as you need to explicitly specify -M
>> q35. But then changing the default machine from the existing pc to q35
>> would break the command line.
>
> That will happen _anyway_.  No doubt there will be configurations which
> work with -M pc and break with -M q35.

Yup.

Without -M, you get whatever the developer picked for you.

If we took backward compatibility dogmatically serious, the default
would still be pc-0.10.  Fortunately, we don't.  We pick something we
think makes sense for most users, in particular casual users unaware of
-M.  Whether and when that will become some q35 machine will be debated
when q35 works.

If you want the best possible compatibility, use -M FOO, where FOO is
whatever machine type you've been using (machine type, not alias,
i.e. -M pc-1.2, not -M pc).



[Qemu-devel] [PATCH 5/9] ehci: Kick async schedule on wakeup in the non companion case

2012-07-09 Thread Gerd Hoffmann
From: Hans de Goede 

Commit 0f588df8b3688b00e77aabaa32e26ece5f19bd39, added code
to ehci_wakeup to kick the async schedule on wakeup, but the else
was positioned wrong making it trigger for devices which are routed
to the companion rather then to the ehci controller itself.

This patch fixes this. Note that the "programming style" with using the
return at the end of the companion block matches how the companion case
is handled in the other ports ops, and is done this way for consistency.

Signed-off-by: Hans de Goede 
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ehci.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index f612610..080f62c 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -905,10 +905,11 @@ static void ehci_wakeup(USBPort *port)
 USBPort *companion = s->companion_ports[port->index];
 if (companion->ops->wakeup) {
 companion->ops->wakeup(companion);
-} else {
-qemu_bh_schedule(s->async_bh);
 }
+return;
 }
+
+qemu_bh_schedule(s->async_bh);
 }
 
 static int ehci_register_companion(USBBus *bus, USBPort *ports[],
-- 
1.7.1




[Qemu-devel] [PATCH v3 04/10] esp: implement Reset ATN command

2012-07-09 Thread Hervé Poussineau

Signed-off-by: Hervé Poussineau 
---
 hw/esp.c |4 
 trace-events |1 +
 2 files changed, 5 insertions(+)

diff --git a/hw/esp.c b/hw/esp.c
index 985a2ee..85078e0 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -113,6 +113,7 @@ struct ESPState {
 #define CMD_MSGACC   0x12
 #define CMD_PAD  0x18
 #define CMD_SATN 0x1a
+#define CMD_RSTATN   0x1b
 #define CMD_SEL  0x41
 #define CMD_SELATN   0x42
 #define CMD_SELATNS  0x43
@@ -634,6 +635,9 @@ static void esp_mem_write(void *opaque, target_phys_addr_t 
addr,
 case CMD_SATN:
 trace_esp_mem_writeb_cmd_satn(val);
 break;
+case CMD_RSTATN:
+trace_esp_mem_writeb_cmd_rstatn(val);
+break;
 case CMD_SEL:
 trace_esp_mem_writeb_cmd_sel(val);
 handle_s_without_atn(s);
diff --git a/trace-events b/trace-events
index ba14f75..0488e34 100644
--- a/trace-events
+++ b/trace-events
@@ -670,6 +670,7 @@ esp_mem_writeb_cmd_iccs(uint32_t val) "Initiator Command 
Complete Sequence (%2.2
 esp_mem_writeb_cmd_msgacc(uint32_t val) "Message Accepted (%2.2x)"
 esp_mem_writeb_cmd_pad(uint32_t val) "Transfer padding (%2.2x)"
 esp_mem_writeb_cmd_satn(uint32_t val) "Set ATN (%2.2x)"
+esp_mem_writeb_cmd_rstatn(uint32_t val) "Reset ATN (%2.2x)"
 esp_mem_writeb_cmd_sel(uint32_t val) "Select without ATN (%2.2x)"
 esp_mem_writeb_cmd_selatn(uint32_t val) "Select with ATN (%2.2x)"
 esp_mem_writeb_cmd_selatns(uint32_t val) "Select with ATN & stop (%2.2x)"
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 3/5] target-i386: call x86_cpu_realize() after APIC is initialized.

2012-07-09 Thread igor

On 06/20/2012 03:35 PM, Andreas Färber wrote:

Am 20.06.2012 14:59, schrieb Igor Mammedov:

It's not correct to make CPU runnable (i.e. calling x86_cpu_realize())
when not all properties are set (APIC in this case).

Fix it by calling x86_cpu_realize() at board level after APIC is
initialized, right before cpu_reset().

Signed-off-by: Igor Mammedov 
---
  hw/pc.c  |1 +
  target-i386/helper.c |2 --
  2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 8368701..8a662cf 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -948,6 +948,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
  env->apic_state = apic_init(env, env->cpuid_apic_id);
  }
  qemu_register_reset(pc_cpu_reset, cpu);
+x86_cpu_realize(OBJECT(cpu), NULL);
  pc_cpu_reset(cpu);
  return cpu;
  }
diff --git a/target-i386/helper.c b/target-i386/helper.c
index c52ec13..b38ea7f 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1161,8 +1161,6 @@ X86CPU *cpu_x86_init(const char *cpu_model)
  return NULL;
  }

-x86_cpu_realize(OBJECT(cpu), NULL);
-
  return cpu;
  }



This will require changes in linux-user and possibly bsd-user. Having a
cpu_realize() would probably help with avoiding #ifdef'ery.
Unfortunately deriving CPUState from DeviceState proves a bit difficult
in the meantime (it worked at one point, now there's lots of circular
header dependencies), and realize support for Object got stopped.

Andreas

As alternative to keep, I could leave x86_cpu_realize() in 
cpu_x86_init() and keep pc_cpu_reset() in pc_new_cpu(). That will result 
in calling cpu_reset() 3 instead of 2 times.
Later when apic_init is moved inside cpu.c, a pc_cpu_reset() in 
pc_new_cpu() would be unnecessary and could be cleaned up then.






[Qemu-devel] [PATCH v3 07/10] esp: split esp code into generic chip emulation and sysbus layer

2012-07-09 Thread Hervé Poussineau

Signed-off-by: Hervé Poussineau 
---
 hw/esp.c |  162 --
 1 file changed, 95 insertions(+), 67 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index d9dd2aa..796cdc1 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -44,12 +44,9 @@
 typedef struct ESPState ESPState;
 
 struct ESPState {
-SysBusDevice busdev;
-MemoryRegion iomem;
 uint8_t rregs[ESP_REGS];
 uint8_t wregs[ESP_REGS];
 qemu_irq irq;
-uint32_t it_shift;
 uint8_t chip_id;
 int32_t ti_size;
 uint32_t ti_rptr, ti_wptr;
@@ -166,11 +163,8 @@ static void esp_lower_irq(ESPState *s)
 }
 }
 
-static void esp_dma_enable(void *opaque, int irq, int level)
+static void esp_dma_enable(ESPState *s, int irq, int level)
 {
-DeviceState *d = opaque;
-ESPState *s = container_of(d, ESPState, busdev.qdev);
-
 if (level) {
 s->dma_enabled = 1;
 trace_esp_dma_enable();
@@ -470,10 +464,8 @@ static void handle_ti(ESPState *s)
 }
 }
 
-static void esp_hard_reset(DeviceState *d)
+static void esp_hard_reset(ESPState *s)
 {
-ESPState *s = container_of(d, ESPState, busdev.qdev);
-
 memset(s->rregs, 0, ESP_REGS);
 memset(s->wregs, 0, ESP_REGS);
 s->rregs[ESP_TCHI] = s->chip_id;
@@ -487,40 +479,23 @@ static void esp_hard_reset(DeviceState *d)
 s->rregs[ESP_CFG1] = 7;
 }
 
-static void esp_soft_reset(DeviceState *d)
+static void esp_soft_reset(ESPState *s)
 {
-ESPState *s = container_of(d, ESPState, busdev.qdev);
-
 qemu_irq_lower(s->irq);
-esp_hard_reset(d);
+esp_hard_reset(s);
 }
 
-static void parent_esp_reset(void *opaque, int irq, int level)
+static void parent_esp_reset(ESPState *s, int irq, int level)
 {
 if (level) {
-esp_soft_reset(opaque);
+esp_soft_reset(s);
 }
 }
 
-static void esp_gpio_demux(void *opaque, int irq, int level)
+static uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
 {
-switch (irq) {
-case 0:
-parent_esp_reset(opaque, irq, level);
-break;
-case 1:
-esp_dma_enable(opaque, irq, level);
-break;
-}
-}
+uint32_t old_val;
 
-static uint64_t esp_mem_read(void *opaque, target_phys_addr_t addr,
- unsigned size)
-{
-ESPState *s = opaque;
-uint32_t saddr, old_val;
-
-saddr = addr >> s->it_shift;
 trace_esp_mem_readb(saddr, s->rregs[saddr]);
 switch (saddr) {
 case ESP_FIFO:
@@ -556,13 +531,8 @@ static uint64_t esp_mem_read(void *opaque, 
target_phys_addr_t addr,
 return s->rregs[saddr];
 }
 
-static void esp_mem_write(void *opaque, target_phys_addr_t addr,
-  uint64_t val, unsigned size)
+static void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val)
 {
-ESPState *s = opaque;
-uint32_t saddr;
-
-saddr = addr >> s->it_shift;
 trace_esp_mem_writeb(saddr, s->wregs[saddr], val);
 switch (saddr) {
 case ESP_TCLO:
@@ -602,7 +572,7 @@ static void esp_mem_write(void *opaque, target_phys_addr_t 
addr,
 break;
 case CMD_RESET:
 trace_esp_mem_writeb_cmd_reset(val);
-esp_soft_reset(&s->busdev.qdev);
+esp_soft_reset(s);
 break;
 case CMD_BUSRESET:
 trace_esp_mem_writeb_cmd_bus_reset(val);
@@ -688,13 +658,6 @@ static bool esp_mem_accepts(void *opaque, 
target_phys_addr_t addr,
 return (size == 1) || (is_write && size == 4);
 }
 
-static const MemoryRegionOps esp_mem_ops = {
-.read = esp_mem_read,
-.write = esp_mem_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
-.valid.accepts = esp_mem_accepts,
-};
-
 static const VMStateDescription vmstate_esp = {
 .name ="esp",
 .version_id = 3,
@@ -717,6 +680,40 @@ static const VMStateDescription vmstate_esp = {
 }
 };
 
+typedef struct {
+SysBusDevice busdev;
+MemoryRegion iomem;
+uint32_t it_shift;
+ESPState esp;
+} SysBusESPState;
+
+static void sysbus_esp_mem_write(void *opaque, target_phys_addr_t addr,
+ uint64_t val, unsigned int size)
+{
+SysBusESPState *sysbus = opaque;
+uint32_t saddr;
+
+saddr = addr >> sysbus->it_shift;
+esp_reg_write(&sysbus->esp, saddr, val);
+}
+
+static uint64_t sysbus_esp_mem_read(void *opaque, target_phys_addr_t addr,
+unsigned int size)
+{
+SysBusESPState *sysbus = opaque;
+uint32_t saddr;
+
+saddr = addr >> sysbus->it_shift;
+return esp_reg_read(&sysbus->esp, saddr);
+}
+
+static const MemoryRegionOps sysbus_esp_mem_ops = {
+.read = sysbus_esp_mem_read,
+.write = sysbus_esp_mem_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid.accepts = esp_mem_accepts,
+};
+
 void esp_init(target_phys_addr_t espaddr, int it_shift,
   ESPDMAMemoryReadWriteFunc dma_memory_read,
   ESPDMAMemoryReadWriteFunc dma_memory_write,
@@ -725,14 +722,16 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
 {
 De

[Qemu-devel] [PATCH 2/9] ehci: fix td writeback

2012-07-09 Thread Gerd Hoffmann
Only write back the dwords the hc is supposed to update.  Should not
make a difference in theory as the guest must not touch the td while
it is active to avoid races.  But it is still more correct.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ehci.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 7de47e5..1406b84 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2070,6 +2070,7 @@ out:
 static int ehci_state_writeback(EHCIQueue *q)
 {
 EHCIPacket *p = QTAILQ_FIRST(&q->packets);
+uint32_t *qtd, addr;
 int again = 0;
 
 /*  Write back the QTD from the QH area */
@@ -2077,8 +2078,9 @@ static int ehci_state_writeback(EHCIQueue *q)
 assert(p->qtdaddr == q->qtdaddr);
 
 ehci_trace_qtd(q, NLPTR_GET(p->qtdaddr), (EHCIqtd *) &q->qh.next_qtd);
-put_dwords(q->ehci, NLPTR_GET(p->qtdaddr), (uint32_t *) &q->qh.next_qtd,
-   sizeof(EHCIqtd) >> 2);
+qtd = (uint32_t *) &q->qh.next_qtd;
+addr = NLPTR_GET(p->qtdaddr);
+put_dwords(q->ehci, addr + 2 * sizeof(uint32_t), qtd + 2, 2);
 ehci_free_packet(p);
 
 /*
-- 
1.7.1




[Qemu-devel] [PATCH 4/9] usb-ehci: Fix an assert whenever isoc transfers are used

2012-07-09 Thread Gerd Hoffmann
From: Hans de Goede 

hcd-ehci.c is missing an usb_packet_init() call for the ipacket UsbPacket
it uses for isoc transfers, triggering an assert (taking the entire vm down)
in usb_packet_setup as soon as any isoc transfers are done by a high speed
USB device.

Signed-off-by: Hans de Goede 
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ehci.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 0bdfbe8..f612610 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2581,6 +2581,7 @@ static int usb_ehci_initfn(PCIDevice *dev)
 s->async_bh = qemu_bh_new(ehci_async_bh, s);
 QTAILQ_INIT(&s->aqueues);
 QTAILQ_INIT(&s->pqueues);
+usb_packet_init(&s->ipacket);
 
 qemu_register_reset(ehci_reset, s);
 
-- 
1.7.1




[Qemu-devel] [PATCH v3 01/10] esp: execute select commands immediately when it is a non-dma command

2012-07-09 Thread Hervé Poussineau

Signed-off-by: Hervé Poussineau 
---
 hw/esp.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 8d73e56..aff8de6 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -270,7 +270,7 @@ static void handle_satn(ESPState *s)
 uint8_t buf[32];
 int len;
 
-if (!s->dma_enabled) {
+if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_satn;
 return;
 }
@@ -284,7 +284,7 @@ static void handle_s_without_atn(ESPState *s)
 uint8_t buf[32];
 int len;
 
-if (!s->dma_enabled) {
+if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_s_without_atn;
 return;
 }
@@ -296,7 +296,7 @@ static void handle_s_without_atn(ESPState *s)
 
 static void handle_satn_stop(ESPState *s)
 {
-if (!s->dma_enabled) {
+if (s->dma && !s->dma_enabled) {
 s->dma_cb = handle_satn_stop;
 return;
 }
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 0/2] usb/scsi: usb attached scsi emulation

2012-07-09 Thread Paolo Bonzini
Il 09/07/2012 12:39, Gerd Hoffmann ha scritto:
> On 07/09/12 12:33, Gerd Hoffmann wrote:
>>   Hi,
>>
>> v2 of the usb attached scsi emulation patches.  Patch #1 is almost
>> unmodified compared to v1.  Patch #2 is new and makes UAS emulation
>> use the new free_request callback (patch just posted by paolo) and
>> obviously depends on that patch to compile.
> 
> Pushed with paolos patch and all pending usb bugfixes to
> "git://git.kraxel.org/qemu usb.56" for those who want to play with it.

Feel free to include my patch in a pull request, of course.

Paolo





Re: [Qemu-devel] [PATCH 7/7 v6] deal with panicked event accoring to '-machine panic_action=action'

2012-07-09 Thread Wen Congyang
At 07/06/2012 07:09 PM, Jan Kiszka Wrote:
> On 2012-07-06 11:41, Wen Congyang wrote:
>> The action is the same as -onpanic parameter.
> 
> As explained in patch 5, now that we have a related device, this no
> longer needs to be a machine property.
> 
> Would could be a machine property is enabling/disabling this device.
> That's probably useful as it uses a fixed PIO port that might conflict
> with (non-Linux) guest expectations and/or future device models.

Yes, it is very useful. I will do it.

Thanks
Wen Congyang

> 
> Jan
> 
>>
>> Signed-off-by: Wen Congyang 
>> ---
>>  qemu-config.c   |4 
>>  qemu-options.hx |4 +++-
>>  vl.c|7 +++
>>  3 files changed, 14 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-config.c b/qemu-config.c
>> index 5c3296b..805e7c4 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -595,6 +595,10 @@ static QemuOptsList qemu_machine_opts = {
>>  .name = "dt_compatible",
>>  .type = QEMU_OPT_STRING,
>>  .help = "Overrides the \"compatible\" property of the dt root 
>> node",
>> +}, {
>> +.name = "panic_action",
>> +.type = QEMU_OPT_STRING,
>> +.help = "The action what QEMU will do when the guest is 
>> panicked",
>>  },
>>  { /* End of list */ }
>>  },
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 4a061bf..083a21d 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -33,7 +33,9 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>>  "property accel=accel1[:accel2[:...]] selects 
>> accelerator\n"
>>  "supported accelerators are kvm, xen, tcg (default: 
>> tcg)\n"
>>  "kernel_irqchip=on|off controls accelerated irqchip 
>> support\n"
>> -"kvm_shadow_mem=size of KVM shadow MMU\n",
>> +"kvm_shadow_mem=size of KVM shadow MMU\n"
>> +"panic_action=none|pause|poweroff|reset controls what 
>> QEmu\n"
>> +"will do when the guest is panicked",
>>  QEMU_ARCH_ALL)
>>  STEXI
>>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
>> diff --git a/vl.c b/vl.c
>> index 1a68257..091c43b 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2301,6 +2301,7 @@ int main(int argc, char **argv, char **envp)
>>  };
>>  const char *trace_events = NULL;
>>  const char *trace_file = NULL;
>> +const char *panic_action = NULL;
>>  
>>  atexit(qemu_run_exit_notifiers);
>>  error_set_progname(argv[0]);
>> @@ -3372,10 +3373,16 @@ int main(int argc, char **argv, char **envp)
>>  kernel_filename = qemu_opt_get(machine_opts, "kernel");
>>  initrd_filename = qemu_opt_get(machine_opts, "initrd");
>>  kernel_cmdline = qemu_opt_get(machine_opts, "append");
>> +panic_action = qemu_opt_get(machine_opts, "panic_action");
>>  } else {
>>  kernel_filename = initrd_filename = kernel_cmdline = NULL;
>>  }
>>  
>> +if (panic_action && select_panicked_action(panic_action) == -1) {
>> +fprintf(stderr, "Unknown -panic_action parameter\n");
>> +exit(1);
>> +}
>> +
>>  if (!kernel_cmdline) {
>>  kernel_cmdline = "";
>>  }
>>
> 




Re: [Qemu-devel] [PATCH 0/2] usb/scsi: usb attached scsi emulation

2012-07-09 Thread Gerd Hoffmann
On 07/09/12 12:33, Gerd Hoffmann wrote:
>   Hi,
> 
> v2 of the usb attached scsi emulation patches.  Patch #1 is almost
> unmodified compared to v1.  Patch #2 is new and makes UAS emulation
> use the new free_request callback (patch just posted by paolo) and
> obviously depends on that patch to compile.

Pushed with paolos patch and all pending usb bugfixes to
"git://git.kraxel.org/qemu usb.56" for those who want to play with it.

cheers,
  Gerd




[Qemu-devel] [PATCH 8/9] usb: fix interface initialization

2012-07-09 Thread Gerd Hoffmann
zero is a valid interface number, so don't use it when resetting the
endpoints.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb.h  |2 ++
 hw/usb/core.c |4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/usb.h b/hw/usb.h
index 9cd2f89..7ed8fb8 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -145,6 +145,8 @@
 #define USB_ENDPOINT_XFER_INT  3
 #define USB_ENDPOINT_XFER_INVALID 255
 
+#define USB_INTERFACE_INVALID 255
+
 typedef struct USBBus USBBus;
 typedef struct USBBusOps USBBusOps;
 typedef struct USBPort USBPort;
diff --git a/hw/usb/core.c b/hw/usb/core.c
index fe15be0..0614f76 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -566,8 +566,8 @@ void usb_ep_reset(USBDevice *dev)
 dev->ep_out[ep].pid = USB_TOKEN_OUT;
 dev->ep_in[ep].type = USB_ENDPOINT_XFER_INVALID;
 dev->ep_out[ep].type = USB_ENDPOINT_XFER_INVALID;
-dev->ep_in[ep].ifnum = 0;
-dev->ep_out[ep].ifnum = 0;
+dev->ep_in[ep].ifnum = USB_INTERFACE_INVALID;
+dev->ep_out[ep].ifnum = USB_INTERFACE_INVALID;
 dev->ep_in[ep].dev = dev;
 dev->ep_out[ep].dev = dev;
 dev->ep_in[ep].pipeline = false;
-- 
1.7.1




[Qemu-devel] [PATCH 2/2] uas: use scsi req refcounting + free_request callback

2012-07-09 Thread Gerd Hoffmann
With the new free_request callback for SCSIBusInfo we can use scsi
request refcounting instead of our own for the uas request lifecycle.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/dev-uas.c |   33 ++---
 1 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 21fc80e..9b02ff4 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -123,11 +123,11 @@ struct UASRequest {
 USBPacket*data;
 bool data_async;
 bool active;
+bool complete;
 uint32_t buf_off;
 uint32_t buf_size;
 uint32_t data_off;
 uint32_t data_size;
-uint32_t refcount;
 QTAILQ_ENTRY(UASRequest)  next;
 };
 
@@ -381,7 +381,7 @@ static void usb_uas_start_next_transfer(UASDevice *uas)
 UASRequest *req;
 
 QTAILQ_FOREACH(req, &uas->requests, next) {
-if (req->active) {
+if (req->active || req->complete) {
 continue;
 }
 if (req->req->cmd.mode == SCSI_XFER_FROM_DEV && uas->datain == NULL) {
@@ -408,12 +408,12 @@ static UASRequest *usb_uas_alloc_request(UASDevice *uas, 
uas_ui *ui)
 req->tag = be16_to_cpu(ui->hdr.tag);
 req->lun = be64_to_cpu(ui->command.lun);
 req->dev = usb_uas_get_dev(req->uas, req->lun);
-req->refcount = 1;
 return req;
 }
 
-static void usb_uas_release_request(UASRequest *req)
+static void usb_uas_scsi_free_request(SCSIBus *bus, void *priv)
 {
+UASRequest *req = priv;
 UASDevice *uas = req->uas;
 
 if (req == uas->datain) {
@@ -438,19 +438,6 @@ static UASRequest *usb_uas_find_request(UASDevice *uas, 
uint16_t tag)
 return NULL;
 }
 
-static void usb_uas_ref_request(UASRequest *req)
-{
-req->refcount++;
-}
-
-static void usb_uas_unref_request(UASRequest *req)
-{
-req->refcount--;
-if (req->refcount == 0) {
-usb_uas_release_request(req);
-}
-}
-
 static void usb_uas_scsi_transfer_data(SCSIRequest *r, uint32_t len)
 {
 UASRequest *req = r->hba_private;
@@ -472,13 +459,12 @@ static void usb_uas_scsi_command_complete(SCSIRequest *r,
 UASDevice *uas = req->uas;
 
 trace_usb_uas_scsi_complete(req->uas->dev.addr, req->tag, status, resid);
+req->complete = true;
 if (req->data) {
 usb_uas_complete_data_packet(req);
 }
 usb_uas_queue_sense(req, status);
 scsi_req_unref(req->req);
-req->req = NULL;
-usb_uas_unref_request(req);
 usb_uas_start_next_transfer(uas);
 }
 
@@ -487,7 +473,7 @@ static void usb_uas_scsi_request_cancelled(SCSIRequest *r)
 UASRequest *req = r->hba_private;
 
 /* FIXME: queue notification to status pipe? */
-usb_uas_unref_request(req);
+scsi_req_unref(req->req);
 }
 
 static const struct SCSIBusInfo usb_uas_scsi_info = {
@@ -498,6 +484,7 @@ static const struct SCSIBusInfo usb_uas_scsi_info = {
 .transfer_data = usb_uas_scsi_transfer_data,
 .complete = usb_uas_scsi_command_complete,
 .cancel = usb_uas_scsi_request_cancelled,
+.free_request = usb_uas_scsi_free_request,
 };
 
 /* - */
@@ -706,17 +693,17 @@ static int usb_uas_handle_data(USBDevice *dev, USBPacket 
*p)
 ret = USB_RET_STALL;
 break;
 }
-usb_uas_ref_request(req);
+scsi_req_ref(req->req);
 req->data = p;
 usb_uas_copy_data(req);
-if (p->result == p->iov.size || req->req == NULL) {
+if (p->result == p->iov.size || req->complete) {
 req->data = NULL;
 ret = p->result;
 } else {
 req->data_async = true;
 ret = USB_RET_ASYNC;
 }
-usb_uas_unref_request(req);
+scsi_req_unref(req->req);
 usb_uas_start_next_transfer(uas);
 break;
 default:
-- 
1.7.1




[Qemu-devel] [PATCH 1/2] usb: add usb attached scsi emulation

2012-07-09 Thread Gerd Hoffmann
$subject says all.  First cut.

It's a pure UAS (usb attached scsi) emulation, without BOT (bulk-only
transport) compatibility.  If your guest can't handle it use usb-storage
instead.

The emulation works like any other scsi hba emulation (eps, lsi, virtio,
megasas, ...).  It provides just the HBA where you can attach scsi
devices as you like using '-device'.  A single scsi target with up to
256 luns is supported.

For now only usb 2.0 transport is supported.  This will change in the
future though as I plan to use this as playground when codeing up &
testing usb 3.0 transport and streams support in the qemu usb core and
the xhci emulation.

No migration support yet.  I'm planning to add usb 3.0 support first as
this probably requires saving additional state.

Special thanks go to Paolo for bringing the qemu scsi emulation into
shape, so this can be added nicely without having to touch a single line
of scsi code.

Signed-off-by: Gerd Hoffmann 
---
 docs/usb-storage.txt |   38 +++
 hw/usb/Makefile.objs |1 +
 hw/usb/dev-uas.c |  792 ++
 trace-events |   14 +
 4 files changed, 845 insertions(+), 0 deletions(-)
 create mode 100644 docs/usb-storage.txt
 create mode 100644 hw/usb/dev-uas.c

diff --git a/docs/usb-storage.txt b/docs/usb-storage.txt
new file mode 100644
index 000..ff97559
--- /dev/null
+++ b/docs/usb-storage.txt
@@ -0,0 +1,38 @@
+
+qemu usb storage emulation
+--
+
+Qemu has two emulations for usb storage devices.
+
+Number one emulates the classic bulk-only transport protocol which is
+used by 99% of the usb sticks on the marked today and is called
+"usb-storage".  Usage (hooking up to xhci, other host controllers work
+too):
+
+  qemu ${other_vm_args}\
+   -drive if=none,id=stick,file=/path/to/file.img  \
+   -device nec-usb-xhci,id=xhci\
+   -device usb-storage,bus=xhci.0,drive=stick
+
+
+Number two is the newer usb attached scsi transport.  This one doesn't
+automagically create a scsi disk, so you have to explicitly attach one
+manually.  Multiple logical units are supported.  Here is an example
+with tree logical units:
+
+  qemu ${other_vm_args}\
+   -drive if=none,id=uas-disk1,file=/path/to/file1.img \
+   -drive if=none,id=uas-disk2,file=/path/to/file2.img \
+   -drive if=none,id=uas-cdrom,media=cdrom,file=/path/to/image.iso \
+   -device nec-usb-xhci,id=xhci\
+   -device usb-uas,id=uas,bus=xhci.0   \
+   -device scsi-hd,bus=uas.0,scsi-id=0,lun=0,drive=uas-disk1   \
+   -device scsi-hd,bus=uas.0,scsi-id=0,lun=1,drive=uas-disk2   \
+   -device scsi-cd,bus=uas.0,scsi-id=0,lun=5,drive=uas-cdrom
+
+
+enjoy,
+  Gerd
+
+--
+Gerd Hoffmann 
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 9c7ddf5..4225136 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -11,3 +11,4 @@ common-obj-y += core.o bus.o desc.o dev-hub.o
 common-obj-y += host-$(HOST_USB).o dev-bluetooth.o
 common-obj-y += dev-hid.o dev-storage.o dev-wacom.o
 common-obj-y += dev-serial.o dev-network.o dev-audio.o
+common-obj-y += dev-uas.o
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
new file mode 100644
index 000..21fc80e
--- /dev/null
+++ b/hw/usb/dev-uas.c
@@ -0,0 +1,792 @@
+/*
+ * UAS (USB Attached SCSI) emulation
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Author: Gerd Hoffmann 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "qemu-option.h"
+#include "qemu-config.h"
+#include "trace.h"
+
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/scsi.h"
+#include "hw/scsi-defs.h"
+
+/* - */
+
+#define UAS_UI_COMMAND  0x01
+#define UAS_UI_SENSE0x03
+#define UAS_UI_RESPONSE 0x04
+#define UAS_UI_TASK_MGMT0x05
+#define UAS_UI_READ_READY   0x06
+#define UAS_UI_WRITE_READY  0x07
+
+#define UAS_RC_TMF_COMPLETE 0x00
+#define UAS_RC_INVALID_INFO_UNIT0x02
+#define UAS_RC_TMF_NOT_SUPPORTED0x04
+#define UAS_RC_TMF_FAILED   0x05
+#define UAS_RC_TMF_SUCCEEDED0x08
+#define UAS_RC_INCORRECT_LUN0x09
+#define UAS_RC_OVERLAPPED_TAG   0x0a
+
+#define UAS_TMF_ABORT_TASK  0x01
+#define UAS_TMF_ABORT_TASK_SET  0x02
+#define UAS_TMF_CLEAR_TASK_SET  0x04
+#define UAS_TMF_LOGICAL_UNIT_RESET  0x08
+#define UAS_TMF_I_T_NEXUS_RESET 0x10
+#define UAS_TMF_CLEAR_ACA   0x40
+#define UAS_TMF_QUERY_TASK  0x80
+#define UAS_TMF_QUERY_TASK_SET  0x81
+#define UAS_TMF_QUERY_ASYNC_EVENT   0x82
+
+#define UAS_PIPE_ID_COMMAND 0x01
+#define UAS_PIPE_ID_STATUS

[Qemu-devel] [PATCH 0/2] usb/scsi: usb attached scsi emulation

2012-07-09 Thread Gerd Hoffmann
  Hi,

v2 of the usb attached scsi emulation patches.  Patch #1 is almost
unmodified compared to v1.  Patch #2 is new and makes UAS emulation
use the new free_request callback (patch just posted by paolo) and
obviously depends on that patch to compile.

cheers,
  Gerd

Gerd Hoffmann (2):
  usb: add usb attached scsi emulation
  uas: use scsi req refcounting + free_request callback

 docs/usb-storage.txt |   38 +++
 hw/usb/Makefile.objs |1 +
 hw/usb/dev-uas.c |  779 ++
 trace-events |   14 +
 4 files changed, 832 insertions(+), 0 deletions(-)
 create mode 100644 docs/usb-storage.txt
 create mode 100644 hw/usb/dev-uas.c




<    1   2   3   >