Re: [Qemu-block] [PATCH v2 0/3] qcow2: Do not mark inactive images corrupt

2018-06-09 Thread Max Reitz
On 2018-06-06 21:36, Max Reitz wrote:
> The non-public logs in
> https://bugzilla.redhat.com/show_bug.cgi?id=1583346 (sorry...) reveal
> this problem:
> 
> $ (Create a qcow2 file "foo.qcow2" with a corrupted first L1 entry)
> $ echo 'qemu-io none0 "read 0 512"' \
> | x86_64-softmmu/qemu-system-x86_64 -drive if=none,file=foo.qcow2 \
> -monitor stdio \
> -incoming exec:'cat /dev/null'
> QEMU 2.12.50 monitor - type 'help' for more information
> (qemu) qemu-io none0 "read 0 512"
> qcow2: Marking image as corrupt: L2 table offset 0x44200 unaligned (L1 index: 
> 0); further corruption events will be suppressed
> qemu-system-x86_64: block/io.c:1691: bdrv_co_pwritev: Assertion 
> `!(bs->open_flags & BDRV_O_INACTIVE)' failed.
> [1]18444 done echo 'qemu-io none0 "read 0 512"' |
>18445 abort (core dumped)  x86_64-softmmu/qemu-system-x86_64 -drive 
> if=none,file=foo.qcow2 -monitor stdi
> 
> Oops.
> 
> 
> The first patch in this series makes a function public that the second
> patch uses to fix the issue by treating all non-writable images like
> read-only images (yes, there is a difference...) in this regard (which
> most importantly means not trying to set the corrupt flag on them).
> Inactive images count as non-writable images, but not as read-only
> images, so that fixes it.
> 
> The third patch adds an iotest case.

Thanks for the reviews, applied to my block branch.

Max



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH] throttle: Fix crash on reopen

2018-06-09 Thread Max Reitz
On 2018-06-08 17:15, Alberto Garcia wrote:
> The throttle block filter can be reopened, and with this it is
> possible to change the throttle group that the filter belongs to.
> 
> The way the code does that is the following:
> 
>   - On throttle_reopen_prepare(): create a new ThrottleGroupMember
> and attach it to the new throttle group.
> 
>   - On throttle_reopen_commit(): detach the old ThrottleGroupMember,
> delete it and replace it with the new one.
> 
> The problem with this is that by replacing the ThrottleGroupMember the
> previous value of io_limits_disabled is lost, causing an assertion
> failure in throttle_co_drain_end().
> 
> This problem can be reproduced by reopening a throttle node:
> 
>$QEMU -monitor stdio
>-object throttle-group,id=tg0,x-iops-total=1000 \
>-blockdev 
> node-name=hd0,driver=qcow2,file.driver=file,file.filename=hd.qcow2 \
>-blockdev 
> node-name=root,driver=throttle,throttle-group=tg0,file=hd0,read-only=on
> 
>(qemu) block_stream root
>block/throttle.c:214: throttle_co_drain_end: Assertion 
> `tgm->io_limits_disabled' failed.
> 
> Since we only want to change the throttle group on reopen there's no
> need to create a ThrottleGroupMember and discard the old one. It's
> easier if we simply detach it from its current group and attach it to
> the new one.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/throttle.c | 54 +-
>  1 file changed, 33 insertions(+), 21 deletions(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH] block/qcow2-bitmap: fix free_bitmap_clusters

2018-06-09 Thread Max Reitz
On 2018-06-08 12:12, Vladimir Sementsov-Ogievskiy wrote:
> This assert may fail, because bitmap_table is not initialized. Just
> drop it, as it's obvious, that bitmap_table_load sets bitmap_table
> parameter only when returning zero.
> 
> Reported-by: Pavel Butsykin 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-bitmap.c | 1 -
>  1 file changed, 1 deletion(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [Qemu-devel] storing machine data in qcow images?

2018-06-09 Thread Max Reitz
On 2018-06-07 23:43, Michael S. Tsirkin wrote:
> On Wed, Jun 06, 2018 at 07:06:27PM +0200, Max Reitz wrote:

[...]

>> Er, yeah, OK.  But it was my understanding that we decided that we have
>> a management layer on top of qemu to make things simple.
> 
> Who's we?

Everyone I'm usually talking to when it comes to adding features to the
block layer, be it on the mailing list or at KVM Forum.

Also my own judgment, when I look at how extensively the block layer
uses QMP, which by definition is not meant to be used by humans.

So this is strongly biased by my immediate environment.

>   I don't think the QEMU community completely gave up on people
> using QEMU directly. It will need to be much more user-friendly than it
> is right now.

You are the very first person I hear this from.  Don't take this as
disagreement, it's just that most of the energy recently seem to go into
making qemu interoperate more easily with management layers than with
end users.  Those goals are not necessarily mutually exclusive, but one
thing this changes did was to make "modern" qemu configuration rather
verbose, which is not what I'd call user-friendly.

>   But it's possible. Fabrice built an emulator in
> javascript, you go to a URL bam it runs a VM.

So?

You can still give qemu an image directly and bam it runs a VM.  It's
just that you get an i440 with an IDE HDD, probably like the JS qemu.

The whole point of this thread was that you might want to run q35, and I
don't know, maybe virtio-blk/scsi.  So exactly what you don't get from
the JS VM.

qemu would be very easy to use if it didn't offer any configuration
options.  The problem is that is offers a huge load of configuration
options and it is not reasonable to expect every user to know all of them.

One way to solve this is to add a management layer which knows all of
the options and may choose good defaults based on knowledge that the
qemu process itself doesn't have (i.e. "put implementation in qemu, and
choose the policy in the management layer").  This is at least what the
block layer usually chooses to do.

>> Also, this is once more a case of first deciding what we want at all.
> 
> Who's we here again?

The participants of this discussion.  Maybe I should have said "you"
because it doesn't necessarily concern myself.

OTOH, we don't have an appliance solution so far (as far as I'm aware),
so I suppose it concerns all of the qemu community, and probably
everyone working somewhere up any qemu management stack as well.

>  Different people want different things. Enough
> people seem to want to store tagged data with a disk image that it might
> be worth someone's while to try to add that capability for starters to
> qemu-img.

I disagree, and I will explain why below. [1]

>> Dave wants configuration options for the upper management layer which
>> are completely opaque to qemu.  That has nothing to do whatsoever with
>> the usability of qemu itself.
> 
> That's why I keep saying, let's start with implementing a mechanism,
> worry about policy later if at all.

[1] I wholeheartedly disagree.

Say you have this mechanism now.  Whatever mechanism it is, because
there isn't even any consensus on that (you'd like key-value binary
object storage, Dave just wants a simple strong with some key-value
format that hasn't been defined in detail, although he has given some
proposals).

Then you need to implement something on top of it, store some values,
interpret them somehow.  And this is exactly what I am asking right now.
 What do you want to store?  Where do you want to interpret it?

I do not understand why you think it's harder to decide that now than
after you have extended qcow2.

I absolutely do think designing the qcow2 extension at least becomes
simpler if you do that design now.


There is a case to be made that one shouldn't worry too much about the
future.  Sometimes you just gotta start somewhere and see where it
leads, maybe the initial design was crap, too bad, then you need to
start over.  But at least you got something done.

But we have talked for half a week now.  In my very personal opinion we
definitely haven't reached the point yet where we just need to start
with something.  Appliances would be a big thing, no need to rush it.

[...]

> I think what we are seeing here is many people jumping on the
> bandwagon and finding more and more uses for ability to store
> meta-data in the qcow2 file.

I don't see that here, but it may very well be true that other people
may find it useful even for other purposes than appliances.

> This just means we should make it flexible enough to possibly
> support more uses. It does not mean we need to make it
> read mail on day 1.

So you are saying that we may end up with multiple parties storing
(meta-)data independently in the qcow2 file?

That would be an argument on why we'd want opaque metadata storage
before there are concrete design documents, and on why it doesn't matter

Re: [Qemu-block] [Qemu-devel] [RFC 12/13] dp8393x: put DMA temp buffer in the state, not in the stack

2018-06-09 Thread Thomas Huth
On 08.06.2018 22:05, Laurent Vivier wrote:
> It's only 32 bytes, and this simplifies the dp8393x_get()/
> dp8393x_put() interface.

Maybe not worth the effort ... or do you need this in a later patch,
too? If so, please mention it in the patch description here.

> Signed-off-by: Laurent Vivier 
> ---
>  hw/net/dp8393x.c | 107 
> ++-
>  1 file changed, 51 insertions(+), 56 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 5061474e6b..40e5f8257b 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -168,6 +168,7 @@ typedef struct dp8393xState {
>  
>  /* Temporaries */
>  uint8_t tx_buffer[0x1];
> +uint16_t data[16];

Why 16? The biggest array that you replaced has only 12 entries...

Also, while you're at it, maybe change the name of the variable
("dma_data"?) or add a comment with a short explanation ?

 Thomas



Re: [Qemu-block] [Qemu-devel] [RFC 11/13] dp8393x: manage big endian bus

2018-06-09 Thread Thomas Huth
On 08.06.2018 22:05, Laurent Vivier wrote:
> This is needed by Quadra 800, this card can run on little-endian
> or big-endian bus.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  hw/net/dp8393x.c | 101 
> ++-
>  1 file changed, 70 insertions(+), 31 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index ef5f1eb94f..5061474e6b 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -150,6 +150,7 @@ typedef struct dp8393xState {
>  
>  /* Hardware */
>  uint8_t it_shift;
> +bool big_endian;
>  qemu_irq irq;
>  #ifdef DEBUG_SONIC
>  int irq_level;
> @@ -174,6 +175,12 @@ typedef struct dp8393xState {
>  AddressSpace as;
>  } dp8393xState;
>  
> +#ifdef HOST_WORDS_BIGENDIAN
> +static const bool host_big_endian = true;
> +#else
> +static const bool host_big_endian = false;
> +#endif
> +
>  /* Accessor functions for values which are formed by
>   * concatenating two 16 bit device registers. By putting these
>   * in their own functions with a uint32_t return type we avoid the
> @@ -220,6 +227,36 @@ static uint32_t dp8393x_wt(dp8393xState *s)
>  return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0];
>  }
>  
> +static uint16_t dp8393x_get(dp8393xState *s, int width, uint16_t *base,
> +int offset)
> +{
> +uint16_t val;
> +
> +if (s->big_endian) {
> +val = base[offset * width + width - 1];
> +} else {
> +val = base[offset * width];
> +}
> +if (s->big_endian != host_big_endian) {
> +val = bswap16(val);
> +}
> +return val;
> +}

Could you maybe write that like this instead:

{
uint16_t val;

if (s->big_endian) {
val = base[offset * width + width - 1];
val = be16_to_cpu(val);
} else {
val = base[offset * width];
val = le16_to_cpu(val);
}
return val;
}

?
... then you don't need that ugly host_big_endian variable anymore.

 Thomas



Re: [Qemu-block] [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-09 Thread Thomas Huth
On 09.06.2018 16:25, Philippe Mathieu-Daudé wrote:
> Hi Laurent,
> 
> On 06/08/2018 05:05 PM, Laurent Vivier wrote:
>> if you want to test the machine, I'm sorry, it doesn't boot
>> a MacROM, but you can boot a linux kernel from the command line.
>>
>> You can install your own disk using debian-installer, with:
>>
>> ...
>> -M q800 \
>> -serial none -serial mon:stdio \
>> -m 1000M -drive file=m68k.qcow2,format=qcow2 \
>> -net nic,model=dp83932,addr=09:00:07:12:34:57 \
>> -append "console=ttyS0 vga=off" \
>> -kernel vmlinux-4.15.0-2-m68k \
>> -initrd initrd.gz \
>> -drive file=debian-9.0-m68k-NETINST-1.iso \
>> -drive file=m68k.qcow2,format=qcow2 \
>> -nographic
>>
>> If you use a graphic adapter instead of "-nographic", you can use "-g" to 
>> set the
>> size of the display (I use "-g 1600x800x24").
>>
>> You can get the ISO from:
>>
>> https://cdimage.debian.org/mirror/cdimage/ports/9.0/m68k/iso-cd/debian-9.0-m68k-NETINST-1.iso
>>
>> and extract the kernel and initrd.gz:
>>
>> guestfish --add debian-9.0-m68k-NETINST-1.iso --ro \
>>   --mount /dev/sda:/ <<_EOF_
>> copy-out /install/cdrom/initrd.gz .
>> copy-out /install/kernels/vmlinux-4.15.0-2-m68k .
>> _EOF_
> 
> Running with -d in_asm,int I get:
> 
> 
> IN: nf_get_id
> 0xd432:  movel %a3,%d0
> 0xd434:  addil #0,%d0
> 0xd43a:  movel %d0,%sp@-
> 0xd43c:  jsr 0xd404
> 
> 
> IN:
> 0xd404:  071400
> 
> INT  1: Unassigned(0xf4) pc=d404 sp=00393e60 sr=2700
> INT  2: Access Fault(0x8) pc= sp=00393e58 sr=2700
> ssw:  0506 ea:    sfc:  5dfc: 5
> 
> 
> IN:
> 0x280c:  clrl %sp@-
> 0x280e:  pea 0x
> 0x2812:  movel %d0,%sp@-
> 0x2814:  moveml %d1-%d5/%a0-%a2,%sp@-
> 0x2818:  movel %sp,%d0
> 0x281a:  andil #-8192,%d0
> 0x2820:  moveal %d0,%a2
> 0x2822:  moveal %a2@,%a2
> 0x2824:  movel %sp,%sp@-
> 0x2826:  bsrl 0x557c
> 
> 
> IN: buserr_c
> 0x557c:  subql #4,%sp
> 0x557e:  moveml %d2-%d7/%a3-%fp,%sp@-
> 0x5582:  moveal %sp@(48),%a3
> 0x5586:  btst #5,%a3@(44)
> 0x558c:  bnes 0x5592
> 
> ...
> 
> 
> IN: panic
> 0x0002c956:  moveal 0x39503c,%a0
> 0x0002c95c:  moveq #101,%d1
> 0x0002c95e:  subql #1,%d1
> 0x0002c960:  bnes 0x2c9c6
> 
> objdump -S gives:
> 
> d404 :
> d404:   7300mvsb %d0,%d1
> d406:   4e75rts
> 
> Instruction which exists in the disas code, but doesn't seem
> tcg-implemented:
> 
> disas/m68k.c:3654:{"mvsb", 2,   one(0070400),   one(0170700), "*bDd",
> mcfisa_b },

0x7300 is the illegal opcode that is used by the Aranym emulator for its
"Native Feature" (some kind of Hypercall) interface:

https://github.com/aranym/aranym/wiki/natfeats-proposal#special-opcodes

It's also a valid opcode in the ColdFire ISA (that's why the
disassembler detects this as valid instruction), but the name of the
function (nf_get_id_phys) clearly indicates that Linux is trying to use
the Natfeats opcode here.

So it's normal that this opcode is not implemented in QEMU 680x0 mode.
If Linux correctly catches the illegal opcode exception afterwards,
everything is fine and you don't need to worry about this anymore.
However, if Linux fails to catch it correctly, there is certainly
something wrong here...

 Thomas



[Qemu-block] [PATCH v3 06/11] block/nbd-client: move from quit to state

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
To implement reconnect we need several states for the client:
CONNECTED, QUIT and several CONNECTING states. CONNECTING states will
be realized in the following patches. This patch implements CONNECTED
and QUIT.

QUIT means, that we should close the connection and fail all current
and further requests (like old quit = true).

CONNECTED means that connection is ok, we can send requests (like old
quit = false).

For receiving loop we use a comparison of the current state with QUIT,
because reconnect will be in the same loop, so it should be looping
until the end.

Opposite, for requests we use a comparison of the current state with
CONNECTED, as we don't want to send requests in CONNECTING states (
which are unreachable now, but will be reachable after the following
commits)

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h | 10 +-
 block/nbd-client.c | 55 --
 2 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index a93f2114b9..014015cd7e 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -23,6 +23,14 @@ typedef struct {
 bool receiving; /* waiting for read_reply_co? */
 } NBDClientRequest;
 
+typedef enum NBDClientState {
+NBD_CLIENT_CONNECTING_INIT,
+NBD_CLIENT_CONNECTING_WAIT,
+NBD_CLIENT_CONNECTING_NOWAIT,
+NBD_CLIENT_CONNECTED,
+NBD_CLIENT_QUIT
+} NBDClientState;
+
 typedef struct NBDClientSession {
 QIOChannelSocket *sioc; /* The master data channel */
 QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
@@ -32,10 +40,10 @@ typedef struct NBDClientSession {
 CoQueue free_sema;
 Coroutine *read_reply_co;
 int in_flight;
+NBDClientState state;
 
 NBDClientRequest requests[MAX_NBD_REQUESTS];
 NBDReply reply;
-bool quit;
 } NBDClientSession;
 
 NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 1589ceb475..7a644e482f 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -34,6 +34,12 @@
 #define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
 #define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
 
+/* @ret would be used for reconnect in future */
+static void nbd_channel_error(NBDClientSession *s, int ret)
+{
+s->state = NBD_CLIENT_QUIT;
+}
+
 static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
 {
 int i;
@@ -73,14 +79,15 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 int ret = 0;
 Error *local_err = NULL;
 
-while (!s->quit) {
+while (s->state != NBD_CLIENT_QUIT) {
 assert(s->reply.handle == 0);
 ret = nbd_receive_reply(s->ioc, >reply, _err);
 if (local_err) {
 error_report_err(local_err);
 }
 if (ret <= 0) {
-break;
+nbd_channel_error(s, ret ? ret : -EIO);
+continue;
 }
 
 /* There's no need for a mutex on the receive side, because the
@@ -93,7 +100,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 !s->requests[i].receiving ||
 (nbd_reply_is_structured(>reply) && !s->info.structured_reply))
 {
-break;
+nbd_channel_error(s, -EINVAL);
+continue;
 }
 
 /* We're woken up again by the request itself.  Note that there
@@ -111,7 +119,6 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 qemu_coroutine_yield();
 }
 
-s->quit = true;
 nbd_recv_coroutines_wake_all(s);
 s->read_reply_co = NULL;
 }
@@ -121,12 +128,18 @@ static int nbd_co_send_request(BlockDriverState *bs,
QEMUIOVector *qiov)
 {
 NBDClientSession *s = nbd_get_client_session(bs);
-int rc, i;
+int rc, i = -1;
 
 qemu_co_mutex_lock(>send_mutex);
 while (s->in_flight == MAX_NBD_REQUESTS) {
 qemu_co_queue_wait(>free_sema, >send_mutex);
 }
+
+if (s->state != NBD_CLIENT_CONNECTED) {
+rc = -EIO;
+goto err;
+}
+
 s->in_flight++;
 
 for (i = 0; i < MAX_NBD_REQUESTS; i++) {
@@ -144,16 +157,12 @@ static int nbd_co_send_request(BlockDriverState *bs,
 
 request->handle = INDEX_TO_HANDLE(s, i);
 
-if (s->quit) {
-rc = -EIO;
-goto err;
-}
 assert(s->ioc);
 
 if (qiov) {
 qio_channel_set_cork(s->ioc, true);
 rc = nbd_send_request(s->ioc, request);
-if (rc >= 0 && !s->quit) {
+if (rc >= 0 && s->state == NBD_CLIENT_CONNECTED) {
 if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
NULL) < 0) {
 rc = -EIO;
@@ -168,9 +177,11 @@ static int nbd_co_send_request(BlockDriverState *bs,
 
 err:
 if (rc < 0) {
-s->quit = true;
-s->requests[i].coroutine = NULL;
-s->in_flight--;
+

[Qemu-block] [PATCH v5 3/6] nbd/server: add nbd_meta_empty_or_pattern helper

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Add nbd_meta_pattern() and nbd_meta_empty_or_pattern() helpers for
metadata query parsing. nbd_meta_pattern() will be reused for "qemu"
namespace in following patches.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/server.c | 86 +---
 1 file changed, 59 insertions(+), 27 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 567561a77e..2d762d7289 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -733,52 +733,83 @@ static int nbd_negotiate_send_meta_context(NBDClient 
*client,
 return qio_channel_writev_all(client->ioc, iov, 2, errp) < 0 ? -EIO : 0;
 }
 
-/* nbd_meta_base_query
- *
- * Handle query to 'base' namespace. For now, only base:allocation context is
- * available in it.  'len' is the amount of text remaining to be read from
- * the current name, after the 'base:' portion has been stripped.
+/* Read strlen(@pattern) bytes, and set @match to true if they match @pattern.
+ * @match is never set to false.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
  * sending a reply about inconsistent lengths, or 1 on success.
  *
- * Note: return code = 1 doesn't mean that we've parsed "base:allocation"
- * namespace. It only means that there are no errors.*/
-static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
-   uint32_t len, Error **errp)
+ * Note: return code = 1 doesn't mean that we've read exactly @pattern
+ * It only means that there are no errors. */
+static int nbd_meta_pattern(NBDClient *client, const char *pattern, bool 
*match,
+Error **errp)
 {
 int ret;
-char query[sizeof("allocation") - 1];
-size_t alen = strlen("allocation");
+char *query;
+int len = strlen(pattern);
 
-if (len == 0) {
-if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
-meta->base_allocation = true;
-}
-trace_nbd_negotiate_meta_query_parse("base:");
-return 1;
-}
-
-if (len != alen) {
-trace_nbd_negotiate_meta_query_skip("not base:allocation");
-return nbd_opt_skip(client, len, errp);
-}
+assert(len);
 
+query = g_malloc(len);
 ret = nbd_opt_read(client, query, len, errp);
 if (ret <= 0) {
+g_free(query);
 return ret;
 }
 
-if (strncmp(query, "allocation", alen) == 0) {
-trace_nbd_negotiate_meta_query_parse("base:allocation");
-meta->base_allocation = true;
+if (strncmp(query, pattern, len) == 0) {
+trace_nbd_negotiate_meta_query_parse(pattern);
+*match = true;
 } else {
-trace_nbd_negotiate_meta_query_skip("not base:allocation");
+trace_nbd_negotiate_meta_query_skip(pattern);
 }
+g_free(query);
 
 return 1;
 }
 
+/* Read @len bytes, and set @match to true if they match @pattern, or if @len
+ * is 0 and the client is performing _LIST_. @match is never set to false.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success.
+ *
+ * Note: return code = 1 doesn't mean that we've read exactly @pattern
+ * It only means that there are no errors. */
+static int nbd_meta_empty_or_pattern(NBDClient *client, const char *pattern,
+ uint32_t len, bool *match, Error **errp)
+{
+if (len == 0) {
+if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
+*match = true;
+}
+trace_nbd_negotiate_meta_query_parse("empty");
+return 1;
+}
+
+if (len != strlen(pattern)) {
+trace_nbd_negotiate_meta_query_skip("different lengths");
+return nbd_opt_skip(client, len, errp);
+}
+
+return nbd_meta_pattern(client, pattern, match, errp);
+}
+
+/* nbd_meta_base_query
+ *
+ * Handle query to 'base' namespace. For now, only base:allocation context is
+ * available in it.  'len' is the amount of text remaining to be read from
+ * the current name, after the 'base:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
+   uint32_t len, Error **errp)
+{
+return nbd_meta_empty_or_pattern(client, "allocation", len,
+ >base_allocation, errp);
+}
+
 /* nbd_negotiate_meta_query
  *
  * Parse namespace name and call corresponding function to parse body of the
@@ -822,6 +853,7 @@ static int nbd_negotiate_meta_query(NBDClient *client,
 return nbd_opt_skip(client, len, errp);
 }
 
+trace_nbd_negotiate_meta_query_parse("base:");
 return nbd_meta_base_query(client, meta, len, errp);
 }
 
-- 
2.11.1




[Qemu-block] [PATCH v3 03/11] block/nbd-client: split connection from initialization

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Split connection code to reuse it for reconnect.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 6ff505c4b8..14b42f31df 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -999,12 +999,12 @@ static QIOChannelSocket 
*nbd_establish_connection(SocketAddress *saddr,
 return sioc;
 }
 
-int nbd_client_init(BlockDriverState *bs,
-SocketAddress *saddr,
-const char *export,
-QCryptoTLSCreds *tlscreds,
-const char *hostname,
-Error **errp)
+static int nbd_client_connect(BlockDriverState *bs,
+  SocketAddress *saddr,
+  const char *export,
+  QCryptoTLSCreds *tlscreds,
+  const char *hostname,
+  Error **errp)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 int ret;
@@ -1048,8 +1048,6 @@ int nbd_client_init(BlockDriverState *bs,
 bs->supported_zero_flags |= BDRV_REQ_MAY_UNMAP;
 }
 
-qemu_co_mutex_init(>send_mutex);
-qemu_co_queue_init(>free_sema);
 client->sioc = sioc;
 
 if (!client->ioc) {
@@ -1066,3 +1064,18 @@ int nbd_client_init(BlockDriverState *bs,
 logout("Established connection with NBD server\n");
 return 0;
 }
+
+int nbd_client_init(BlockDriverState *bs,
+SocketAddress *saddr,
+const char *export,
+QCryptoTLSCreds *tlscreds,
+const char *hostname,
+Error **errp)
+{
+NBDClientSession *client = nbd_get_client_session(bs);
+
+qemu_co_mutex_init(>send_mutex);
+qemu_co_queue_init(>free_sema);
+
+return nbd_client_connect(bs, saddr, export, tlscreds, hostname, errp);
+}
-- 
2.11.1




[Qemu-block] [PATCH v5 2/6] nbd/server: refactor NBDExportMetaContexts

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Use NBDExport pointer instead of just export name: there no needs to
store duplicated name in the struct, moreover, NBDExport will be used
further.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/server.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 8e02e077ec..567561a77e 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -88,7 +88,7 @@ static QTAILQ_HEAD(, NBDExport) exports = 
QTAILQ_HEAD_INITIALIZER(exports);
  * as selected by NBD_OPT_SET_META_CONTEXT. Also used for
  * NBD_OPT_LIST_META_CONTEXT. */
 typedef struct NBDExportMetaContexts {
-char export_name[NBD_MAX_NAME_SIZE + 1];
+NBDExport *exp;
 bool valid; /* means that negotiation of the option finished without
errors */
 bool base_allocation; /* export base:allocation context (block status) */
@@ -399,10 +399,9 @@ static int nbd_negotiate_handle_list(NBDClient *client, 
Error **errp)
 return nbd_negotiate_send_rep(client, NBD_REP_ACK, errp);
 }
 
-static void nbd_check_meta_export_name(NBDClient *client)
+static void nbd_check_meta_export(NBDClient *client)
 {
-client->export_meta.valid &= !strcmp(client->exp->name,
- client->export_meta.export_name);
+client->export_meta.valid &= client->exp == client->export_meta.exp;
 }
 
 /* Send a reply to NBD_OPT_EXPORT_NAME.
@@ -456,7 +455,7 @@ static int nbd_negotiate_handle_export_name(NBDClient 
*client,
 
 QTAILQ_INSERT_TAIL(>exp->clients, client, next);
 nbd_export_get(client->exp);
-nbd_check_meta_export_name(client);
+nbd_check_meta_export(client);
 
 return 0;
 }
@@ -650,7 +649,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, 
uint16_t myflags,
 client->exp = exp;
 QTAILQ_INSERT_TAIL(>exp->clients, client, next);
 nbd_export_get(client->exp);
-nbd_check_meta_export_name(client);
+nbd_check_meta_export(client);
 rc = 1;
 }
 return rc;
@@ -834,7 +833,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
   NBDExportMetaContexts *meta, Error 
**errp)
 {
 int ret;
-NBDExport *exp;
+char export_name[NBD_MAX_NAME_SIZE + 1];
 NBDExportMetaContexts local_meta;
 uint32_t nb_queries;
 int i;
@@ -853,15 +852,15 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
 
 memset(meta, 0, sizeof(*meta));
 
-ret = nbd_opt_read_name(client, meta->export_name, NULL, errp);
+ret = nbd_opt_read_name(client, export_name, NULL, errp);
 if (ret <= 0) {
 return ret;
 }
 
-exp = nbd_export_find(meta->export_name);
-if (exp == NULL) {
+meta->exp = nbd_export_find(export_name);
+if (meta->exp == NULL) {
 return nbd_opt_drop(client, NBD_REP_ERR_UNKNOWN, errp,
-"export '%s' not present", meta->export_name);
+"export '%s' not present", export_name);
 }
 
 ret = nbd_opt_read(client, _queries, sizeof(nb_queries), errp);
@@ -870,7 +869,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
 }
 cpu_to_be32s(_queries);
 trace_nbd_negotiate_meta_context(nbd_opt_lookup(client->opt),
- meta->export_name, nb_queries);
+ export_name, nb_queries);
 
 if (client->opt == NBD_OPT_LIST_META_CONTEXT && !nb_queries) {
 /* enable all known contexts */
-- 
2.11.1




[Qemu-block] [PATCH v3 01/11] block/nbd-client: split channel errors from export errors

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
To implement nbd reconnect in further patches, we need to distinguish
error codes, returned by nbd server, from channel errors, to reconnect
only in the latter case.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 83 +++---
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 8d69eaaa32..9b9a82fef1 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -501,11 +501,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
  */
 static coroutine_fn int nbd_co_receive_one_chunk(
 NBDClientSession *s, uint64_t handle, bool only_structured,
-QEMUIOVector *qiov, NBDReply *reply, void **payload, Error **errp)
+int *request_ret, QEMUIOVector *qiov, NBDReply *reply, void **payload,
+Error **errp)
 {
-int request_ret;
 int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured,
-  _ret, qiov, payload, errp);
+  request_ret, qiov, payload, errp);
 
 if (ret < 0) {
 s->quit = true;
@@ -515,7 +515,6 @@ static coroutine_fn int nbd_co_receive_one_chunk(
 *reply = s->reply;
 }
 s->reply.handle = 0;
-ret = request_ret;
 }
 
 if (s->read_reply_co) {
@@ -527,22 +526,17 @@ static coroutine_fn int nbd_co_receive_one_chunk(
 
 typedef struct NBDReplyChunkIter {
 int ret;
-bool fatal;
+int request_ret;
 Error *err;
 bool done, only_structured;
 } NBDReplyChunkIter;
 
-static void nbd_iter_error(NBDReplyChunkIter *iter, bool fatal,
-   int ret, Error **local_err)
+static void nbd_iter_channel_error(NBDReplyChunkIter *iter,
+   int ret, Error **local_err)
 {
 assert(ret < 0);
 
-if ((fatal && !iter->fatal) || iter->ret == 0) {
-if (iter->ret != 0) {
-error_free(iter->err);
-iter->err = NULL;
-}
-iter->fatal = fatal;
+if (!iter->ret) {
 iter->ret = ret;
 error_propagate(>err, *local_err);
 } else {
@@ -552,6 +546,15 @@ static void nbd_iter_error(NBDReplyChunkIter *iter, bool 
fatal,
 *local_err = NULL;
 }
 
+static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
+{
+assert(ret < 0);
+
+if (!iter->request_ret) {
+iter->request_ret = ret;
+}
+}
+
 /* NBD_FOREACH_REPLY_CHUNK
  */
 #define NBD_FOREACH_REPLY_CHUNK(s, iter, handle, structured, \
@@ -567,13 +570,13 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession 
*s,
  QEMUIOVector *qiov, NBDReply *reply,
  void **payload)
 {
-int ret;
+int ret, request_ret;
 NBDReply local_reply;
 NBDStructuredReplyChunk *chunk;
 Error *local_err = NULL;
 if (s->quit) {
 error_setg(_err, "Connection closed");
-nbd_iter_error(iter, true, -EIO, _err);
+nbd_iter_channel_error(iter, -EIO, _err);
 goto break_loop;
 }
 
@@ -587,10 +590,12 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession 
*s,
 }
 
 ret = nbd_co_receive_one_chunk(s, handle, iter->only_structured,
-   qiov, reply, payload, _err);
+   _ret, qiov, reply, payload,
+   _err);
 if (ret < 0) {
-/* If it is a fatal error s->quit is set by nbd_co_receive_one_chunk */
-nbd_iter_error(iter, s->quit, ret, _err);
+nbd_iter_channel_error(iter, ret, _err);
+} else if (request_ret < 0) {
+nbd_iter_request_error(iter, request_ret);
 }
 
 /* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */
@@ -627,7 +632,7 @@ break_loop:
 }
 
 static int nbd_co_receive_return_code(NBDClientSession *s, uint64_t handle,
-  Error **errp)
+  int *request_ret, Error **errp)
 {
 NBDReplyChunkIter iter;
 
@@ -636,12 +641,13 @@ static int nbd_co_receive_return_code(NBDClientSession 
*s, uint64_t handle,
 }
 
 error_propagate(errp, iter.err);
+*request_ret = iter.request_ret;
 return iter.ret;
 }
 
 static int nbd_co_receive_cmdread_reply(NBDClientSession *s, uint64_t handle,
 uint64_t offset, QEMUIOVector *qiov,
-Error **errp)
+int *request_ret, Error **errp)
 {
 NBDReplyChunkIter iter;
 NBDReply reply;
@@ -666,7 +672,7 @@ static int nbd_co_receive_cmdread_reply(NBDClientSession 
*s, uint64_t handle,
 offset, qiov, _err);
 if (ret < 0) {
 s->quit = true;
-nbd_iter_error(, true, ret, _err);
+nbd_iter_channel_error(, ret, _err);
  

[Qemu-block] [PATCH v5 1/6] nbd/server: fix trace

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Return code = 1 doesn't mean that we parsed base:allocation. Use
correct traces in both -parsed and -skipped cases.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/server.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 9e1f227178..8e02e077ec 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -741,7 +741,10 @@ static int nbd_negotiate_send_meta_context(NBDClient 
*client,
  * the current name, after the 'base:' portion has been stripped.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
- * sending a reply about inconsistent lengths, or 1 on success. */
+ * sending a reply about inconsistent lengths, or 1 on success.
+ *
+ * Note: return code = 1 doesn't mean that we've parsed "base:allocation"
+ * namespace. It only means that there are no errors.*/
 static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
uint32_t len, Error **errp)
 {
@@ -768,10 +771,12 @@ static int nbd_meta_base_query(NBDClient *client, 
NBDExportMetaContexts *meta,
 }
 
 if (strncmp(query, "allocation", alen) == 0) {
+trace_nbd_negotiate_meta_query_parse("base:allocation");
 meta->base_allocation = true;
+} else {
+trace_nbd_negotiate_meta_query_skip("not base:allocation");
 }
 
-trace_nbd_negotiate_meta_query_parse("base:allocation");
 return 1;
 }
 
-- 
2.11.1




[Qemu-block] [PATCH v3 07/11] block/nbd-client: rename read_reply_co to connection_co

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
This coroutine will serve nbd reconnects, so, rename it to be something
more generic.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |  4 ++--
 block/nbd-client.c | 24 
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 014015cd7e..7cb31e72e6 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -20,7 +20,7 @@
 typedef struct {
 Coroutine *coroutine;
 uint64_t offset;/* original offset of the request */
-bool receiving; /* waiting for read_reply_co? */
+bool receiving; /* waiting for connection_co? */
 } NBDClientRequest;
 
 typedef enum NBDClientState {
@@ -38,7 +38,7 @@ typedef struct NBDClientSession {
 
 CoMutex send_mutex;
 CoQueue free_sema;
-Coroutine *read_reply_co;
+Coroutine *connection_co;
 int in_flight;
 NBDClientState state;
 
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 7a644e482f..32c6f531de 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -63,7 +63,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 qio_channel_shutdown(client->ioc,
  QIO_CHANNEL_SHUTDOWN_BOTH,
  NULL);
-BDRV_POLL_WHILE(bs, client->read_reply_co);
+BDRV_POLL_WHILE(bs, client->connection_co);
 
 nbd_client_detach_aio_context(bs);
 object_unref(OBJECT(client->sioc));
@@ -72,7 +72,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 client->ioc = NULL;
 }
 
-static coroutine_fn void nbd_read_reply_entry(void *opaque)
+static coroutine_fn void nbd_connection_entry(void *opaque)
 {
 NBDClientSession *s = opaque;
 uint64_t i;
@@ -105,14 +105,14 @@ static coroutine_fn void nbd_read_reply_entry(void 
*opaque)
 }
 
 /* We're woken up again by the request itself.  Note that there
- * is no race between yielding and reentering read_reply_co.  This
+ * is no race between yielding and reentering connection_co.  This
  * is because:
  *
  * - if the request runs on the same AioContext, it is only
  *   entered after we yield
  *
  * - if the request runs on a different AioContext, reentering
- *   read_reply_co happens through a bottom half, which can only
+ *   connection_co happens through a bottom half, which can only
  *   run after we yield.
  */
 aio_co_wake(s->requests[i].coroutine);
@@ -120,7 +120,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 }
 
 nbd_recv_coroutines_wake_all(s);
-s->read_reply_co = NULL;
+s->connection_co = NULL;
 }
 
 static int nbd_co_send_request(BlockDriverState *bs,
@@ -428,7 +428,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
 }
 *request_ret = 0;
 
-/* Wait until we're woken up by nbd_read_reply_entry.  */
+/* Wait until we're woken up by nbd_connection_entry.  */
 s->requests[i].receiving = true;
 qemu_coroutine_yield();
 s->requests[i].receiving = false;
@@ -503,7 +503,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
 }
 
 /* nbd_co_receive_one_chunk
- * Read reply, wake up read_reply_co and set s->quit if needed.
+ * Read reply, wake up connection_co and set s->quit if needed.
  * Return value is a fatal error code or normal nbd reply error code
  */
 static coroutine_fn int nbd_co_receive_one_chunk(
@@ -517,15 +517,15 @@ static coroutine_fn int nbd_co_receive_one_chunk(
 if (ret < 0) {
 nbd_channel_error(s, ret);
 } else {
-/* For assert at loop start in nbd_read_reply_entry */
+/* For assert at loop start in nbd_connection_entry */
 if (reply) {
 *reply = s->reply;
 }
 s->reply.handle = 0;
 }
 
-if (s->read_reply_co) {
-aio_co_wake(s->read_reply_co);
+if (s->connection_co) {
+aio_co_wake(s->connection_co);
 }
 
 return ret;
@@ -966,7 +966,7 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 qio_channel_attach_aio_context(QIO_CHANNEL(client->ioc), new_context);
-aio_co_schedule(new_context, client->read_reply_co);
+aio_co_schedule(new_context, client->connection_co);
 }
 
 void nbd_client_close(BlockDriverState *bs)
@@ -1063,7 +1063,7 @@ static int nbd_client_connect(BlockDriverState *bs,
 /* Now that we're connected, set the socket to be non-blocking and
  * kick the reply mechanism.  */
 qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
-client->read_reply_co = qemu_coroutine_create(nbd_read_reply_entry, 
client);
+client->connection_co = qemu_coroutine_create(nbd_connection_entry, 
client);
 nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs));
 
 logout("Established connection with NBD server\n");
-- 
2.11.1




[Qemu-block] [PATCH v3 05/11] block/nbd-client: don't check ioc

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
We have several paranoiac checks for ioc != NULL. But ioc may become
NULL only on close, which should not happen during requests handling.
Also, we check ioc only sometimes, not after each yield, which is
inconsistent. Let's drop these checks. However, for safety, lets leave
asserts instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index dd712c59b3..1589ceb475 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -51,9 +51,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 
-if (!client->ioc) { /* Already closed */
-return;
-}
+assert(client->ioc);
 
 /* finish any pending coroutines */
 qio_channel_shutdown(client->ioc,
@@ -150,10 +148,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
 rc = -EIO;
 goto err;
 }
-if (!s->ioc) {
-rc = -EPIPE;
-goto err;
-}
+assert(s->ioc);
 
 if (qiov) {
 qio_channel_set_cork(s->ioc, true);
@@ -426,10 +421,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
 s->requests[i].receiving = true;
 qemu_coroutine_yield();
 s->requests[i].receiving = false;
-if (!s->ioc || s->quit) {
+if (s->quit) {
 error_setg(errp, "Connection closed");
 return -EIO;
 }
+assert(s->ioc);
 
 assert(s->reply.handle == handle);
 
@@ -967,9 +963,7 @@ void nbd_client_close(BlockDriverState *bs)
 NBDClientSession *client = nbd_get_client_session(bs);
 NBDRequest request = { .type = NBD_CMD_DISC };
 
-if (client->ioc == NULL) {
-return;
-}
+assert(client->ioc);
 
 nbd_send_request(client->ioc, );
 
-- 
2.11.1




[Qemu-block] [PATCH v3 04/11] block/nbd-client: fix nbd_reply_chunk_iter_receive

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Use exported report, not the variable to be reused (should not really
matter).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 14b42f31df..dd712c59b3 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -599,7 +599,7 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession 
*s,
 }
 
 /* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */
-if (nbd_reply_is_simple(>reply) || s->quit) {
+if (nbd_reply_is_simple(reply) || s->quit) {
 goto break_loop;
 }
 
-- 
2.11.1




[Qemu-block] [PATCH v3 10/11] block/nbd-client: nbd reconnect

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Implement reconnect. To achieve this:

1. Move from quit bool variable to state. 4 states are introduced:
   connecting-wait: means, that reconnecting is in progress, and there
 were small number of reconnect attempts, so all requests are
 waiting for the connection.
   connecting-nowait: reconnecting is in progress, there were a lot of
 attempts of reconnect, all requests will return errors.
   connected: normal state
   quit: exiting after fatal error or on close

Possible transitions are:

   * -> quit
   connecting-* -> connected
   connecting-wait -> connecting-nowait
   connected -> connecting-wait

2. Implement reconnect in connection_co. So, in connecting-* mode,
connection_co, tries to reconnect every NBD_RECONNECT_NS.
Configuring of this parameter (as well as NBD_RECONNECT_ATTEMPTS,
which specifies bound of transition from connecting-wait to
connecting-nowait) may be done as a follow-up patch.

3. Retry nbd queries on channel error, if we are in connecting-wait
state.

4. In init, wait until for connection until transition to
connecting-nowait. So, NBD_RECONNECT_ATTEMPTS is a bound of fail
for initial connection too.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |   2 +
 block/nbd-client.c | 170 ++---
 2 files changed, 123 insertions(+), 49 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 2561e1ea42..1249f2eb52 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -44,6 +44,8 @@ typedef struct NBDClientSession {
 bool receiving;
 int connect_status;
 Error *connect_err;
+int connect_attempts;
+bool wait_in_flight;
 
 NBDClientRequest requests[MAX_NBD_REQUESTS];
 NBDReply reply;
diff --git a/block/nbd-client.c b/block/nbd-client.c
index f22ed7f404..49b1f67047 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -41,10 +41,16 @@ static int nbd_client_connect(BlockDriverState *bs,
   const char *hostname,
   Error **errp);
 
-/* @ret would be used for reconnect in future */
 static void nbd_channel_error(NBDClientSession *s, int ret)
 {
-s->state = NBD_CLIENT_QUIT;
+if (ret == -EIO) {
+if (s->state == NBD_CLIENT_CONNECTED) {
+s->state = NBD_CLIENT_CONNECTING_WAIT;
+s->connect_attempts = 0;
+}
+} else {
+s->state = NBD_CLIENT_QUIT;
+}
 }
 
 static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
@@ -90,6 +96,19 @@ typedef struct NBDConnection {
 uint64_t reconnect_timeout;
 } NBDConnection;
 
+static bool nbd_client_connecting(NBDClientSession *client)
+{
+return client->state == NBD_CLIENT_CONNECTING_WAIT ||
+   client->state == NBD_CLIENT_CONNECTING_NOWAIT ||
+   client->state == NBD_CLIENT_CONNECTING_INIT;
+}
+
+static bool nbd_client_connecting_wait(NBDClientSession *client)
+{
+return client->state == NBD_CLIENT_CONNECTING_WAIT ||
+   client->state == NBD_CLIENT_CONNECTING_INIT;
+}
+
 static coroutine_fn void nbd_connection_entry(void *opaque)
 {
 NBDConnection *con = opaque;
@@ -98,26 +117,55 @@ static coroutine_fn void nbd_connection_entry(void *opaque)
 int ret = 0;
 Error *local_err = NULL;
 
-if (con->reconnect_attempts != 0) {
-error_setg(>connect_err, "Reconnect is not supported yet");
-s->connect_status = -EINVAL;
-nbd_channel_error(s, s->connect_status);
-return;
-}
+while (s->state != NBD_CLIENT_QUIT) {
+assert(s->reply.handle == 0);
 
-s->connect_status = nbd_client_connect(con->bs, con->saddr,
-   con->export, con->tlscreds,
-   con->hostname, >connect_err);
-if (s->connect_status < 0) {
-nbd_channel_error(s, s->connect_status);
-return;
-}
+if (nbd_client_connecting(s)) {
+if (s->connect_attempts == con->reconnect_attempts) {
+s->state = NBD_CLIENT_CONNECTING_NOWAIT;
+qemu_co_queue_restart_all(>free_sema);
+}
 
-/* successfully connected */
-s->state = NBD_CLIENT_CONNECTED;
+qemu_co_mutex_lock(>send_mutex);
+
+while (s->in_flight > 0) {
+qemu_co_mutex_unlock(>send_mutex);
+nbd_recv_coroutines_wake_all(s);
+s->wait_in_flight = true;
+qemu_coroutine_yield();
+s->wait_in_flight = false;
+qemu_co_mutex_lock(>send_mutex);
+}
+
+qemu_co_mutex_unlock(>send_mutex);
+
+/* Now we are sure, that nobody accessing the channel now and 
nobody
+ * will try to access the channel, until we set state to CONNECTED
+ */
+
+s->connect_status = nbd_client_connect(con->bs, con->saddr,
+   

[Qemu-block] [PATCH v3 00/11] NBD reconnect

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Hi all.

Here is NBD reconnect.
The feature realized inside nbd-client driver and works as follows:

There are two parameters: reconnect-attempts and reconnect-timeout.
So, we will try to reconnect in case of initial connection failed or
in case of connection lost. All current and new io operations will wait
until we make reconnect-attempts tries to reconnect. After this, all
requests will fail with EIO, but we will continue trying to reconnect.

v3:
06: fix build error in function 'nbd_co_send_request':
 error: 'i' may be used uninitialized in this function

v2 notes:
Here is v2 of NBD reconnect, but it is very very different from v1, so,
forget about v1.
The series includes my "NBD reconnect: preliminary refactoring", with
changes in 05: leave asserts (Eric).

Vladimir Sementsov-Ogievskiy (11):
  block/nbd-client: split channel errors from export errors
  block/nbd: move connection code from block/nbd to block/nbd-client
  block/nbd-client: split connection from initialization
  block/nbd-client: fix nbd_reply_chunk_iter_receive
  block/nbd-client: don't check ioc
  block/nbd-client: move from quit to state
  block/nbd-client: rename read_reply_co to connection_co
  block/nbd-client: move connecting to connection_co
  block/nbd: add cmdline and qapi parameters for nbd reconnect
  block/nbd-client: nbd reconnect
  iotests: test nbd reconnect

 qapi/block-core.json  |  12 +-
 block/nbd-client.h|  23 ++-
 block/nbd-client.c| 429 ++
 block/nbd.c   |  61 +++---
 tests/qemu-iotests/220|  68 +++
 tests/qemu-iotests/220.out|   7 +
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |   4 +
 8 files changed, 445 insertions(+), 160 deletions(-)
 create mode 100755 tests/qemu-iotests/220
 create mode 100644 tests/qemu-iotests/220.out

-- 
2.11.1




[Qemu-block] [PATCH v3 08/11] block/nbd-client: move connecting to connection_co

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
As a first step to nbd reconnect, move connection to connection_co
coroutine.

The key point in this patch is nbd_client_attach_aio_context() change:
We schedule to connection_co only if it is waiting for read from the
channel. We should not schedule it if it is some other yield, and if
it is currently executing (we call nbd_client_attach_aio_context from
connection code).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |  3 +++
 block/nbd-client.c | 58 ++
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 7cb31e72e6..f6c8052573 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -41,6 +41,9 @@ typedef struct NBDClientSession {
 Coroutine *connection_co;
 int in_flight;
 NBDClientState state;
+bool receiving;
+int connect_status;
+Error *connect_err;
 
 NBDClientRequest requests[MAX_NBD_REQUESTS];
 NBDReply reply;
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 32c6f531de..44ac4ebc31 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -34,6 +34,13 @@
 #define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
 #define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
 
+static int nbd_client_connect(BlockDriverState *bs,
+  SocketAddress *saddr,
+  const char *export,
+  QCryptoTLSCreds *tlscreds,
+  const char *hostname,
+  Error **errp);
+
 /* @ret would be used for reconnect in future */
 static void nbd_channel_error(NBDClientSession *s, int ret)
 {
@@ -63,6 +70,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 qio_channel_shutdown(client->ioc,
  QIO_CHANNEL_SHUTDOWN_BOTH,
  NULL);
+client->state = NBD_CLIENT_QUIT;
 BDRV_POLL_WHILE(bs, client->connection_co);
 
 nbd_client_detach_aio_context(bs);
@@ -72,16 +80,38 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 client->ioc = NULL;
 }
 
+typedef struct NBDConnection {
+BlockDriverState *bs;
+SocketAddress *saddr;
+const char *export;
+QCryptoTLSCreds *tlscreds;
+const char *hostname;
+} NBDConnection;
+
 static coroutine_fn void nbd_connection_entry(void *opaque)
 {
-NBDClientSession *s = opaque;
+NBDConnection *con = opaque;
+NBDClientSession *s = nbd_get_client_session(con->bs);
 uint64_t i;
 int ret = 0;
 Error *local_err = NULL;
 
+s->connect_status = nbd_client_connect(con->bs, con->saddr,
+   con->export, con->tlscreds,
+   con->hostname, >connect_err);
+if (s->connect_status < 0) {
+nbd_channel_error(s, s->connect_status);
+return;
+}
+
+/* successfully connected */
+s->state = NBD_CLIENT_CONNECTED;
+
 while (s->state != NBD_CLIENT_QUIT) {
 assert(s->reply.handle == 0);
+s->receiving = true;
 ret = nbd_receive_reply(s->ioc, >reply, _err);
+s->receiving = false;
 if (local_err) {
 error_report_err(local_err);
 }
@@ -966,7 +996,9 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 qio_channel_attach_aio_context(QIO_CHANNEL(client->ioc), new_context);
-aio_co_schedule(new_context, client->connection_co);
+if (client->receiving) {
+aio_co_schedule(new_context, client->connection_co);
+}
 }
 
 void nbd_client_close(BlockDriverState *bs)
@@ -1063,7 +1095,6 @@ static int nbd_client_connect(BlockDriverState *bs,
 /* Now that we're connected, set the socket to be non-blocking and
  * kick the reply mechanism.  */
 qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
-client->connection_co = qemu_coroutine_create(nbd_connection_entry, 
client);
 nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs));
 
 logout("Established connection with NBD server\n");
@@ -1078,9 +1109,28 @@ int nbd_client_init(BlockDriverState *bs,
 Error **errp)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
+NBDConnection *con = g_new(NBDConnection, 1);
+
+con->bs = bs;
+con->saddr = saddr;
+con->export = export;
+con->tlscreds = tlscreds;
+con->hostname = hostname;
 
 qemu_co_mutex_init(>send_mutex);
 qemu_co_queue_init(>free_sema);
+client->state = NBD_CLIENT_CONNECTING_INIT;
 
-return nbd_client_connect(bs, saddr, export, tlscreds, hostname, errp);
+client->connection_co = qemu_coroutine_create(nbd_connection_entry, con);
+aio_co_schedule(bdrv_get_aio_context(bs), client->connection_co);
+BDRV_POLL_WHILE(bs, client->state == NBD_CLIENT_CONNECTING_INIT);
+
+if (client->state != 

[Qemu-block] [PATCH v5 4/6] nbd/server: implement dirty bitmap export

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Handle new NBD meta namespace: "qemu", and corresponding queries:
"qemu:dirty-bitmap:".

With new metadata context negotiated, BLOCK_STATUS query will reply
with dirty-bitmap data, converted to extents. New public function
nbd_export_bitmap selects bitmap to export. For now, only one bitmap
may be exported.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/nbd.h |   6 ++
 nbd/server.c| 271 
 2 files changed, 259 insertions(+), 18 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index fcdcd54502..a653d0fba7 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -234,6 +234,10 @@ enum {
 #define NBD_STATE_HOLE (1 << 0)
 #define NBD_STATE_ZERO (1 << 1)
 
+/* Flags for extents (NBDExtent.flags) of NBD_REPLY_TYPE_BLOCK_STATUS,
+ * for qemu:dirty-bitmap:* meta contexts */
+#define NBD_STATE_DIRTY (1 << 0)
+
 static inline bool nbd_reply_type_is_error(int type)
 {
 return type & (1 << 15);
@@ -315,6 +319,8 @@ void nbd_client_put(NBDClient *client);
 void nbd_server_start(SocketAddress *addr, const char *tls_creds,
   Error **errp);
 
+void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
+   const char *bitmap_export_name, Error **errp);
 
 /* nbd_read
  * Reads @size bytes from @ioc. Returns 0 on success.
diff --git a/nbd/server.c b/nbd/server.c
index 2d762d7289..569e068fc2 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -23,6 +23,12 @@
 #include "nbd-internal.h"
 
 #define NBD_META_ID_BASE_ALLOCATION 0
+#define NBD_META_ID_DIRTY_BITMAP 1
+
+/* NBD_MAX_BITMAP_EXTENTS: 1 mb of extents data. An empirical constant. If need
+ * to increase, note that NBD protocol defines 32 mb as a limit, after which 
the
+ * reply may be considered as a denial of service attack. */
+#define NBD_MAX_BITMAP_EXTENTS (0x10 / 8)
 
 static int system_errno_to_nbd_errno(int err)
 {
@@ -80,6 +86,9 @@ struct NBDExport {
 
 BlockBackend *eject_notifier_blk;
 Notifier eject_notifier;
+
+BdrvDirtyBitmap *export_bitmap;
+char *export_bitmap_context;
 };
 
 static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
@@ -92,6 +101,7 @@ typedef struct NBDExportMetaContexts {
 bool valid; /* means that negotiation of the option finished without
errors */
 bool base_allocation; /* export base:allocation context (block status) */
+bool dirty_bitmap; /* export qemu:dirty-bitmap: */
 } NBDExportMetaContexts;
 
 struct NBDClient {
@@ -810,6 +820,55 @@ static int nbd_meta_base_query(NBDClient *client, 
NBDExportMetaContexts *meta,
  >base_allocation, errp);
 }
 
+/* nbd_meta_bitmap_query
+ *
+ * Handle query to 'qemu:' namespace.
+ * @len is the amount of text remaining to be read from the current name, after
+ * the 'qemu:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta,
+   uint32_t len, Error **errp)
+{
+bool dirty_bitmap = false;
+int dirty_bitmap_len = strlen("dirty-bitmap:");
+int ret;
+
+if (!meta->exp->export_bitmap) {
+return nbd_opt_skip(client, len, errp);
+}
+
+if (len == 0) {
+if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
+meta->dirty_bitmap = true;
+}
+trace_nbd_negotiate_meta_query_parse("empty");
+return 1;
+}
+
+if (len < dirty_bitmap_len) {
+trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
+return nbd_opt_skip(client, len, errp);
+}
+
+len -= dirty_bitmap_len;
+ret = nbd_meta_pattern(client, "dirty-bitmap:", _bitmap, errp);
+if (ret <= 0) {
+return ret;
+}
+if (!dirty_bitmap) {
+trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
+return nbd_opt_skip(client, len, errp);
+}
+
+trace_nbd_negotiate_meta_query_parse("dirty-bitmap:");
+
+return nbd_meta_empty_or_pattern(
+client, meta->exp->export_bitmap_context +
+strlen("qemu:dirty_bitmap:"), len, >dirty_bitmap, errp);
+}
+
 /* nbd_negotiate_meta_query
  *
  * Parse namespace name and call corresponding function to parse body of the
@@ -826,8 +885,10 @@ static int nbd_negotiate_meta_query(NBDClient *client,
 NBDExportMetaContexts *meta, Error **errp)
 {
 int ret;
-char query[sizeof("base:") - 1];
-size_t baselen = strlen("base:");
+size_t ns_len = 5;
+char ns[5]; /* Both 'qemu' and 'base' namespaces have length = 5 including 
a
+   colon. If it's needed to introduce a namespace of the other
+   length, this should be certainly refactored. */
 uint32_t len;
 
 ret = nbd_opt_read(client, , sizeof(len), errp);
@@ -836,25 

[Qemu-block] [PATCH v3 02/11] block/nbd: move connection code from block/nbd to block/nbd-client

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Keep all connection code in one file, to be able to implement reconnect
in further patches.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |  2 +-
 block/nbd-client.c | 37 +++--
 block/nbd.c| 41 ++---
 3 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 0ece76e5af..a93f2114b9 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -41,7 +41,7 @@ typedef struct NBDClientSession {
 NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
 
 int nbd_client_init(BlockDriverState *bs,
-QIOChannelSocket *sock,
+SocketAddress *saddr,
 const char *export_name,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 9b9a82fef1..6ff505c4b8 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -976,8 +976,31 @@ void nbd_client_close(BlockDriverState *bs)
 nbd_teardown_connection(bs);
 }
 
+static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
+  Error **errp)
+{
+QIOChannelSocket *sioc;
+Error *local_err = NULL;
+
+sioc = qio_channel_socket_new();
+qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
+
+qio_channel_socket_connect_sync(sioc,
+saddr,
+_err);
+if (local_err) {
+object_unref(OBJECT(sioc));
+error_propagate(errp, local_err);
+return NULL;
+}
+
+qio_channel_set_delay(QIO_CHANNEL(sioc), false);
+
+return sioc;
+}
+
 int nbd_client_init(BlockDriverState *bs,
-QIOChannelSocket *sioc,
+SocketAddress *saddr,
 const char *export,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
@@ -986,6 +1009,15 @@ int nbd_client_init(BlockDriverState *bs,
 NBDClientSession *client = nbd_get_client_session(bs);
 int ret;
 
+/* establish TCP connection, return error if it fails
+ * TODO: Configurable retry-until-timeout behaviour.
+ */
+QIOChannelSocket *sioc = nbd_establish_connection(saddr, errp);
+
+if (!sioc) {
+return -ECONNREFUSED;
+}
+
 /* NBD handshake */
 logout("session init %s\n", export);
 qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL);
@@ -998,12 +1030,14 @@ int nbd_client_init(BlockDriverState *bs,
 >ioc, >info, errp);
 if (ret < 0) {
 logout("Failed to negotiate with the NBD server\n");
+object_unref(OBJECT(sioc));
 return ret;
 }
 if (client->info.flags & NBD_FLAG_READ_ONLY &&
 !bdrv_is_read_only(bs)) {
 error_setg(errp,
"request for write access conflicts with read-only export");
+object_unref(OBJECT(sioc));
 return -EACCES;
 }
 if (client->info.flags & NBD_FLAG_SEND_FUA) {
@@ -1017,7 +1051,6 @@ int nbd_client_init(BlockDriverState *bs,
 qemu_co_mutex_init(>send_mutex);
 qemu_co_queue_init(>free_sema);
 client->sioc = sioc;
-object_ref(OBJECT(client->sioc));
 
 if (!client->ioc) {
 client->ioc = QIO_CHANNEL(sioc);
diff --git a/block/nbd.c b/block/nbd.c
index ff8333e3c1..a851b8cd68 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -305,30 +305,6 @@ NBDClientSession *nbd_get_client_session(BlockDriverState 
*bs)
 return >client;
 }
 
-static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
-  Error **errp)
-{
-QIOChannelSocket *sioc;
-Error *local_err = NULL;
-
-sioc = qio_channel_socket_new();
-qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
-
-qio_channel_socket_connect_sync(sioc,
-saddr,
-_err);
-if (local_err) {
-object_unref(OBJECT(sioc));
-error_propagate(errp, local_err);
-return NULL;
-}
-
-qio_channel_set_delay(QIO_CHANNEL(sioc), false);
-
-return sioc;
-}
-
-
 static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
 {
 Object *obj;
@@ -398,7 +374,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 BDRVNBDState *s = bs->opaque;
 QemuOpts *opts = NULL;
 Error *local_err = NULL;
-QIOChannelSocket *sioc = NULL;
 QCryptoTLSCreds *tlscreds = NULL;
 const char *hostname = NULL;
 int ret = -EINVAL;
@@ -438,22 +413,10 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 hostname = s->saddr->u.inet.host;
 }
 
-/* establish TCP connection, return error if it fails
- * TODO: Configurable retry-until-timeout behaviour.
- */
-sioc = 

[Qemu-block] [PATCH v5 6/6] docs/interop: add nbd.txt

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Describe new metadata namespace: "qemu".

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 docs/interop/nbd.txt | 37 +
 MAINTAINERS  |  1 +
 2 files changed, 38 insertions(+)
 create mode 100644 docs/interop/nbd.txt

diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
new file mode 100644
index 00..7366269fc0
--- /dev/null
+++ b/docs/interop/nbd.txt
@@ -0,0 +1,37 @@
+Qemu supports NBD protocol, and has internal NBD client (look at
+block/nbd.c), internal NBD server (look at blockdev-nbd.c) as well as
+external NBD server tool - qemu-nbd.c. The common code is placed in
+nbd/*.
+
+NBD protocol is specified here:
+https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md
+
+This following paragraphs describe some specific properties of NBD
+protocol realization in Qemu.
+
+
+= Metadata namespaces =
+
+Qemu supports "base:allocation" metadata context as defined in the NBD
+protocol specification and defines own metadata namespace: "qemu".
+
+
+== "qemu" namespace ==
+
+For now, the only type of metadata context in the namespace is dirty
+bitmap. All available metadata contexts have the following form:
+
+   qemu:dirty-bitmap:
+
+Each dirty-bitmap metadata context defines the only one flag for
+extents in reply for NBD_CMD_BLOCK_STATUS:
+
+bit 0: NBD_STATE_DIRTY, means that the extent is "dirty"
+
+For NBD_OPT_LIST_META_CONTEXT the following queries are supported
+additionally to "qemu:dirty-bitmap:":
+
+* "qemu:" : returns list of all available metadata contexts in the
+namespace.
+* "qemu:dirty-bitmap:" : returns list of all available dirty-bitmap
+ metadata contexts.
diff --git a/MAINTAINERS b/MAINTAINERS
index e187b1f18f..887b479440 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1923,6 +1923,7 @@ F: nbd/
 F: include/block/nbd*
 F: qemu-nbd.*
 F: blockdev-nbd.c
+F: docs/interop/nbd.txt
 T: git git://repo.or.cz/qemu/ericb.git nbd
 
 NFS
-- 
2.11.1




[Qemu-block] [PATCH v5 5/6] qapi: new qmp command nbd-server-add-bitmap

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/block.json | 23 +++
 blockdev-nbd.c  | 23 +++
 2 files changed, 46 insertions(+)

diff --git a/qapi/block.json b/qapi/block.json
index c694524002..ddbca2e286 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -269,6 +269,29 @@
   'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
 
 ##
+# @nbd-server-add-bitmap:
+#
+# Expose a dirty bitmap associated with the selected export. The bitmap search
+# starts at the device attached to the export, and includes all backing files.
+# The exported bitmap is then locked until the NBD export is removed.
+#
+# @name: Export name.
+#
+# @bitmap: Bitmap name to search for.
+#
+# @bitmap-export-name: How the bitmap will be seen by nbd clients
+#  (default @bitmap)
+#
+# Note: the client must use NBD_OPT_SET_META_CONTEXT with a query of
+# "qemu:dirty-bitmap:NAME" (where NAME matches @bitmap-export-name) to access
+# the exposed bitmap.
+#
+# Since: 3.0
+##
+  { 'command': 'nbd-server-add-bitmap',
+'data': {'name': 'str', 'bitmap': 'str', '*bitmap-export-name': 'str'} }
+
+##
 # @nbd-server-stop:
 #
 # Stop QEMU's embedded NBD server, and unregister all devices previously
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 65a84739ed..6b0c50732c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -220,3 +220,26 @@ void qmp_nbd_server_stop(Error **errp)
 nbd_server_free(nbd_server);
 nbd_server = NULL;
 }
+
+void qmp_nbd_server_add_bitmap(const char *name, const char *bitmap,
+   bool has_bitmap_export_name,
+   const char *bitmap_export_name,
+   Error **errp)
+{
+NBDExport *exp;
+
+if (!nbd_server) {
+error_setg(errp, "NBD server not running");
+return;
+}
+
+exp = nbd_export_find(name);
+if (exp == NULL) {
+error_setg(errp, "Export '%s' is not found", name);
+return;
+}
+
+nbd_export_bitmap(exp, bitmap,
+  has_bitmap_export_name ? bitmap_export_name : bitmap,
+  errp);
+}
-- 
2.11.1




[Qemu-block] [PATCH v3 09/11] block/nbd: add cmdline and qapi parameters for nbd reconnect

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Add two parameters:

reconnect-attempts, which defines maximum number of reconnects, after
  which:
  - open will fail
  - block operations will fail

Note: on open, we actually have reconnect-attempts+1 connection
attempts, the first one is not REconnect.

reconnect-timeout, timeout in nanoseconds between reconnects.

Realization of these options is in the following patch.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/block-core.json | 12 +++-
 block/nbd-client.h   |  2 ++
 block/nbd-client.c   | 13 +
 block/nbd.c  | 22 +-
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4b1de474a9..4abba9d38e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3356,12 +3356,22 @@
 #
 # @tls-creds:   TLS credentials ID
 #
+# @reconnect-attempts: number of connection attempts on disconnect.
+#  Must be >= 0. Default is 0. On nbd disk open, there 
would
+#  be maximum of @reconnect-attempts + 1 total tries to
+#  connect
+#
+# @reconnect-timeout: timeout between reconnect attempts in nanoseconds. 
Default
+# is 10 (one second)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsNbd',
   'data': { 'server': 'SocketAddress',
 '*export': 'str',
-'*tls-creds': 'str' } }
+'*tls-creds': 'str',
+'*reconnect-attempts': 'uint64',
+'*reconnect-timeout': 'uint64' } }
 
 ##
 # @BlockdevOptionsRaw:
diff --git a/block/nbd-client.h b/block/nbd-client.h
index f6c8052573..2561e1ea42 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -56,6 +56,8 @@ int nbd_client_init(BlockDriverState *bs,
 const char *export_name,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
+uint64_t reconnect_attempts,
+uint64_t reconnect_timeout,
 Error **errp);
 void nbd_client_close(BlockDriverState *bs);
 
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 44ac4ebc31..f22ed7f404 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -86,6 +86,8 @@ typedef struct NBDConnection {
 const char *export;
 QCryptoTLSCreds *tlscreds;
 const char *hostname;
+uint64_t reconnect_attempts;
+uint64_t reconnect_timeout;
 } NBDConnection;
 
 static coroutine_fn void nbd_connection_entry(void *opaque)
@@ -96,6 +98,13 @@ static coroutine_fn void nbd_connection_entry(void *opaque)
 int ret = 0;
 Error *local_err = NULL;
 
+if (con->reconnect_attempts != 0) {
+error_setg(>connect_err, "Reconnect is not supported yet");
+s->connect_status = -EINVAL;
+nbd_channel_error(s, s->connect_status);
+return;
+}
+
 s->connect_status = nbd_client_connect(con->bs, con->saddr,
con->export, con->tlscreds,
con->hostname, >connect_err);
@@ -1106,6 +1115,8 @@ int nbd_client_init(BlockDriverState *bs,
 const char *export,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
+uint64_t reconnect_attempts,
+uint64_t reconnect_timeout,
 Error **errp)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
@@ -1116,6 +1127,8 @@ int nbd_client_init(BlockDriverState *bs,
 con->export = export;
 con->tlscreds = tlscreds;
 con->hostname = hostname;
+con->reconnect_attempts = reconnect_attempts;
+con->reconnect_timeout = reconnect_timeout;
 
 qemu_co_mutex_init(>send_mutex);
 qemu_co_queue_init(>free_sema);
diff --git a/block/nbd.c b/block/nbd.c
index a851b8cd68..8c81d4a151 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -364,6 +364,20 @@ static QemuOptsList nbd_runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "ID of the TLS credentials to use",
 },
+{
+.name = "reconnect-attempts",
+.type = QEMU_OPT_NUMBER,
+.help = "Number of connection attempts on disconnect. "
+"Must be >= 0. Default is 0. On nbd disk open, there would 
"
+"be maximum of @reconnect-attempts + 1 total tries to "
+"connect",
+},
+{
+.name = "reconnect-timeout",
+.type = QEMU_OPT_NUMBER,
+.help = "Timeout between reconnect attempts in nanoseconds. "
+"Default is 10 (one second)",
+},
 { /* end of list */ }
 },
 };
@@ -376,6 +390,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 Error *local_err = NULL;
 QCryptoTLSCreds *tlscreds = NULL;
 const char *hostname = NULL;
+uint64_t reconnect_attempts;
+uint64_t reconnect_timeout;
 int ret = 

[Qemu-block] [PATCH v3 11/11] iotests: test nbd reconnect

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Add test, which starts backup to nbd target and restarts nbd server
during backup.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/220| 68 +++
 tests/qemu-iotests/220.out|  7 +
 tests/qemu-iotests/group  |  1 +
 tests/qemu-iotests/iotests.py |  4 +++
 4 files changed, 80 insertions(+)
 create mode 100755 tests/qemu-iotests/220
 create mode 100644 tests/qemu-iotests/220.out

diff --git a/tests/qemu-iotests/220 b/tests/qemu-iotests/220
new file mode 100755
index 00..f1c26a2796
--- /dev/null
+++ b/tests/qemu-iotests/220
@@ -0,0 +1,68 @@
+#!/usr/bin/env python
+#
+# Test nbd reconnect
+#
+# Copyright (c) 2018 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import time
+
+import iotests
+from iotests import qemu_img_create, file_path, qemu_nbd_popen
+
+disk_a, disk_b, nbd_sock = file_path('disk_a', 'disk_b', 'nbd-sock')
+nbd_uri = 'nbd+unix:///exp?socket=' + nbd_sock
+
+qemu_img_create('-f', iotests.imgfmt, disk_a, '5M')
+qemu_img_create('-f', iotests.imgfmt, disk_b, '5M')
+srv = qemu_nbd_popen('-k', nbd_sock, '-x', 'exp', '-f', iotests.imgfmt, disk_b)
+time.sleep(1)
+
+vm = iotests.VM().add_drive(disk_a)
+vm.launch()
+vm.hmp_qemu_io('drive0', 'write 0 5M')
+
+print 'blockdev-add:', vm.qmp('blockdev-add', node_name='backup0', 
driver='raw',
+  file={'driver':'nbd',
+'reconnect-attempts':10,
+'export': 'exp',
+'server': {'type': 'unix',
+   'path': nbd_sock}})
+print 'blockdev-backup:', vm.qmp('blockdev-backup', device='drive0',
+ sync='full', target='backup0')
+
+time.sleep(1)
+print 'Kill NBD server'
+srv.kill()
+
+jobs = vm.qmp('query-block-jobs')['return']
+if jobs and jobs[0]['offset'] < jobs[0]['len']:
+print 'Backup job is still in progress'
+
+time.sleep(1)
+
+print 'Start NBD server'
+srv = qemu_nbd_popen('-k', nbd_sock, '-x', 'exp', '-f', iotests.imgfmt, disk_b)
+
+try:
+e = vm.event_wait('BLOCK_JOB_COMPLETED')
+print e['event'], ':', e['data']
+except:
+pass
+
+print 'blockdev-del:', vm.qmp('blockdev-del', node_name='backup0')
+srv.kill()
+vm.shutdown()
diff --git a/tests/qemu-iotests/220.out b/tests/qemu-iotests/220.out
new file mode 100644
index 00..dae1a49d9f
--- /dev/null
+++ b/tests/qemu-iotests/220.out
@@ -0,0 +1,7 @@
+blockdev-add: {u'return': {}}
+blockdev-backup: {u'return': {}}
+Kill NBD server
+Backup job is still in progress
+Start NBD server
+BLOCK_JOB_COMPLETED : {u'device': u'drive0', u'type': u'backup', u'speed': 0, 
u'len': 5242880, u'offset': 5242880}
+blockdev-del: {u'return': {}}
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 93f93d71ba..cc5a1cf2a4 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -217,3 +217,4 @@
 216 rw auto quick
 218 rw auto quick
 219 rw auto
+220 rw auto quick
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index fdbdd8b300..7071318c9e 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -175,6 +175,10 @@ def qemu_nbd(*args):
 '''Run qemu-nbd in daemon mode and return the parent's exit code'''
 return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
 
+def qemu_nbd_popen(*args):
+'''Run qemu-nbd in daemon mode and return the parent's exit code'''
+return subprocess.Popen(qemu_nbd_args + ['--persistent'] + list(args))
+
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
 '''Return True if two image files are identical'''
 return qemu_img('compare', '-f', fmt1,
-- 
2.11.1




[Qemu-block] [PATCH v5 0/6] NBD export bitmaps

2018-06-09 Thread Vladimir Sementsov-Ogievskiy
Hi all.

This is a proposal and realization of new NBD meta context: qemu.

New possible queries will look like:
qemu:dirty-bitmap:

Mapping from export-bitmap-name to BdrvDirtyBitmap is done through qmp
command nbd-server-add-bitmap. For now, only one bitmap export is
allowed per NBD export, however it may be easily improved if needed 
(we don't have such cases for now)

Client and testing.
I wrote client code for Virtuozzo, but it turned out to be unused,
actually it's used only for tests. We don't have cases, where we need
to import dirty bitmap through qemu nbd-client. All this done for
exporting dirty bitmaps to the third tool. So, I think, it is not worth
refactoring, rebasing and merging client part upstream, if there are no
real usage cases.

v5:
02: fix nbd_check_meta_export s/client->exp/client->export_meta.exp
04: improve bitmap_to_extents and handle NBD_CMD_FLAG_REQ_ONE correctly

v4:
01: add trace for -skipped case
02: new
03: trace -skipped case in nbd_meta_pattern, rebase on 02
04: fix nbd_meta_qemu_query: use meta->exp instead of client->exp, rebase on 02

v3:
01: new
02: rewritten to satisfy changes in 03, drop r-b
03: - fix comments
- rewrite nbd_meta_bitmap_query() and rename it to
  nbd_meta_qemu_query(): parse 'qemu:dirty-bitmaps:' for LIST
  option to represent list of all dirty-bitmap contexts.
- trace points
- s/512/BDRV_SECTOR_SIZE/
  drop TODO comment
04: s/2.13/3.0
05: new

v2:
01 from v1 is dropped: actually, we don't need generic namespace
parsing for now (especially, after moving to qemu: namespace, which has
the same length as base:), lets postpone it.

01: Improve comment wording (Eric), add Eric's r-b
02: improve commit message
move NBD_STATE_DIRTY to header
add comment on NBD_MAX_BITMAP_EXTENTS
remove MAX_EXTENT_LENGTH and instead update add_extents() which
  uses it
use export_bitmap_context instead of export_bitmap_name to reduce
  operations on it
move from qemu-dirty-bitmap to qemu:dirty-bitmap
other way to parse namespace name
handle FLAG_DF
03: Improve specification of new qmp command (Eric)

Vladimir Sementsov-Ogievskiy (6):
  nbd/server: fix trace
  nbd/server: refactor NBDExportMetaContexts
  nbd/server: add nbd_meta_empty_or_pattern helper
  nbd/server: implement dirty bitmap export
  qapi: new qmp command nbd-server-add-bitmap
  docs/interop: add nbd.txt

 docs/interop/nbd.txt |  37 ++
 qapi/block.json  |  23 
 include/block/nbd.h  |   6 +
 blockdev-nbd.c   |  23 
 nbd/server.c | 353 +--
 MAINTAINERS  |   1 +
 6 files changed, 402 insertions(+), 41 deletions(-)
 create mode 100644 docs/interop/nbd.txt

-- 
2.11.1




Re: [Qemu-block] [Qemu-devel] [RFC 01/13] hw/m68k: add via support

2018-06-09 Thread Mark Cave-Ayland

On 09/06/18 11:01, Mark Cave-Ayland wrote:

Yeah, we can certainly remove a huge chunk of this by converting over to 
the mos6522 device. My last set of updates to CUDA a couple of days ago 
are probably the best reference, but I can probably find some time to do 
the basic conversion for you at some point...


BTW is there a particular github branch I should be working from in 
order to attempt this?



ATB,

Mark.



Re: [Qemu-block] [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-09 Thread Philippe Mathieu-Daudé
Hi Laurent,

On 06/08/2018 05:05 PM, Laurent Vivier wrote:
> if you want to test the machine, I'm sorry, it doesn't boot
> a MacROM, but you can boot a linux kernel from the command line.
> 
> You can install your own disk using debian-installer, with:
> 
> ...
> -M q800 \
> -serial none -serial mon:stdio \
> -m 1000M -drive file=m68k.qcow2,format=qcow2 \
> -net nic,model=dp83932,addr=09:00:07:12:34:57 \
> -append "console=ttyS0 vga=off" \
> -kernel vmlinux-4.15.0-2-m68k \
> -initrd initrd.gz \
> -drive file=debian-9.0-m68k-NETINST-1.iso \
> -drive file=m68k.qcow2,format=qcow2 \
> -nographic
> 
> If you use a graphic adapter instead of "-nographic", you can use "-g" to set 
> the
> size of the display (I use "-g 1600x800x24").
> 
> You can get the ISO from:
> 
> https://cdimage.debian.org/mirror/cdimage/ports/9.0/m68k/iso-cd/debian-9.0-m68k-NETINST-1.iso
> 
> and extract the kernel and initrd.gz:
> 
> guestfish --add debian-9.0-m68k-NETINST-1.iso --ro \
>   --mount /dev/sda:/ <<_EOF_
> copy-out /install/cdrom/initrd.gz .
> copy-out /install/kernels/vmlinux-4.15.0-2-m68k .
> _EOF_

Running with -d in_asm,int I get:


IN: nf_get_id
0xd432:  movel %a3,%d0
0xd434:  addil #0,%d0
0xd43a:  movel %d0,%sp@-
0xd43c:  jsr 0xd404


IN:
0xd404:  071400

INT  1: Unassigned(0xf4) pc=d404 sp=00393e60 sr=2700
INT  2: Access Fault(0x8) pc= sp=00393e58 sr=2700
ssw:  0506 ea:    sfc:  5dfc: 5


IN:
0x280c:  clrl %sp@-
0x280e:  pea 0x
0x2812:  movel %d0,%sp@-
0x2814:  moveml %d1-%d5/%a0-%a2,%sp@-
0x2818:  movel %sp,%d0
0x281a:  andil #-8192,%d0
0x2820:  moveal %d0,%a2
0x2822:  moveal %a2@,%a2
0x2824:  movel %sp,%sp@-
0x2826:  bsrl 0x557c


IN: buserr_c
0x557c:  subql #4,%sp
0x557e:  moveml %d2-%d7/%a3-%fp,%sp@-
0x5582:  moveal %sp@(48),%a3
0x5586:  btst #5,%a3@(44)
0x558c:  bnes 0x5592

...


IN: panic
0x0002c956:  moveal 0x39503c,%a0
0x0002c95c:  moveq #101,%d1
0x0002c95e:  subql #1,%d1
0x0002c960:  bnes 0x2c9c6

objdump -S gives:

d404 :
d404:   7300mvsb %d0,%d1
d406:   4e75rts

Instruction which exists in the disas code, but doesn't seem
tcg-implemented:

disas/m68k.c:3654:{"mvsb", 2,   one(0070400),   one(0170700), "*bDd",
mcfisa_b },

> 
> The mirror to use is: http://ftp.ports.debian.org/debian-ports/
> when it fails, continue without boot loader.
> 
> In the same way, you can extract the kernel and the initramfs from the qcow2
> image to use it with "-kernel" and "-initrd":
> 
> guestfish --add m68k.qcow2 --mount /dev/sda2:/ <<_EOF_
> copy-out /boot/vmlinux-4.15.0-2-m68k .
> copy-out /boot/initrd.img-4.15.0-2-m68k .
> _EOF_
> 
> and boot with:
> 
>...
>-append "root=/dev/sda2 rw console=ttyS0 console=tty \
>-kernel vmlinux-4.15.0-2-m68k \
>-initrd initrd.img-4.15.0-2-m68k



Re: [Qemu-block] [Qemu-devel] [RFC 09/13] hw/m68k: define Macintosh Quadra 800

2018-06-09 Thread Mark Cave-Ayland

On 08/06/18 21:05, Laurent Vivier wrote:


From: Laurent Vivier 

Signed-off-by: Laurent Vivier 
---
  default-configs/m68k-softmmu.mak |  12 ++
  hw/display/macfb.c   |  31 ++--
  hw/m68k/Makefile.objs|   6 +-
  hw/m68k/bootinfo.h   |  99 ++
  hw/m68k/mac.c| 384 +++
  hw/nubus/nubus-device.c  |  13 --
  tests/qom-test.c |   5 +
  tests/test-hmp.c |   3 +-
  8 files changed, 519 insertions(+), 34 deletions(-)
  create mode 100644 hw/m68k/bootinfo.h
  create mode 100644 hw/m68k/mac.c

diff --git a/default-configs/m68k-softmmu.mak b/default-configs/m68k-softmmu.mak
index 60f7cdfbf2..1b568be166 100644
--- a/default-configs/m68k-softmmu.mak
+++ b/default-configs/m68k-softmmu.mak
@@ -2,3 +2,15 @@
  
  CONFIG_COLDFIRE=y

  CONFIG_PTIMER=y
+CONFIG_ESCC=y
+CONFIG_FRAMEBUFFER=y
+CONFIG_ADB=y
+CONFIG_MAC_VIA=y
+CONFIG_MAC=y
+CONFIG_SCSI=y
+CONFIG_ESP=y
+CONFIG_ASC=y
+CONFIG_MACFB=y
+CONFIG_NUBUS=y
+CONFIG_DP8393X=y
+CONFIG_SWIM=y
diff --git a/hw/display/macfb.c b/hw/display/macfb.c
index 295fd0fc8a..a3204ab150 100644
--- a/hw/display/macfb.c
+++ b/hw/display/macfb.c
@@ -240,31 +240,28 @@ typedef struct {
  MacfbState macfb;
  } MacfbNubusState;
  
-static int macfb_sysbus_init(SysBusDevice *dev)

+static void macfb_sysbus_realize(DeviceState *dev, Error **errp)
  {
  MacfbState *s =  (dev)->macfb;
  
-macfb_init(DEVICE(dev), s);

-sysbus_init_mmio(dev, >mem_ctrl);
-sysbus_init_mmio(dev, >mem_vram);
-
-return 0;
+macfb_init(dev, s);
+sysbus_init_mmio(SYS_BUS_DEVICE(s), >mem_ctrl);
+sysbus_init_mmio(SYS_BUS_DEVICE(s), >mem_vram);
  }
  
  const uint8_t macfb_rom[] = {

  255, 0, 0, 0,
  };
  
-static int macfb_nubus_init(NubusDevice *dev)

+static void macfb_nubus_realize(DeviceState *dev, Error **errp)
  {
-MacfbState *s = _UPCAST(MacfbNubusState, busdev, dev)->macfb;


DO_UPCAST() - wow that's a blast from the past! Obviously these should 
be handled via proper QOM objects and casts.



+NubusDevice *nubus = NUBUS_DEVICE(dev);
+MacfbState *s = _UPCAST(MacfbNubusState, busdev, nubus)->macfb;
  
-macfb_init(DEVICE(dev), s);

-nubus_add_slot_mmio(dev, DAFB_BASE, >mem_ctrl);
-nubus_add_slot_mmio(dev, VIDEO_BASE, >mem_vram);
-nubus_register_rom(dev, macfb_rom, sizeof(macfb_rom), 1, 9, 0xf);
-
-return 0;
+macfb_init(dev, s);
+nubus_add_slot_mmio(nubus, DAFB_BASE, >mem_ctrl);
+nubus_add_slot_mmio(nubus, VIDEO_BASE, >mem_vram);
+nubus_register_rom(nubus, macfb_rom, sizeof(macfb_rom), 1, 9, 0xf);
  }
  
  static void macfb_sysbus_reset(DeviceState *d)

@@ -296,9 +293,8 @@ static Property macfb_nubus_properties[] = {
  static void macfb_sysbus_class_init(ObjectClass *klass, void *data)
  {
  DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
  
-k->init = macfb_sysbus_init;

+dc->realize = macfb_sysbus_realize;
  dc->desc = "SysBus Macintosh framebuffer";
  dc->reset = macfb_sysbus_reset;
  dc->vmsd = _macfb;
@@ -308,9 +304,8 @@ static void macfb_sysbus_class_init(ObjectClass *klass, 
void *data)
  static void macfb_nubus_class_init(ObjectClass *klass, void *data)
  {
  DeviceClass *dc = DEVICE_CLASS(klass);
-NubusDeviceClass *k = NUBUS_DEVICE_CLASS(klass);
  
-k->init = macfb_nubus_init;

+dc->realize = macfb_nubus_realize;
  dc->desc = "Nubus Macintosh framebuffer";
  dc->reset = macfb_nubus_reset;
  dc->vmsd = _macfb;
diff --git a/hw/m68k/Makefile.objs b/hw/m68k/Makefile.objs
index d1f089c08a..ff739617b2 100644
--- a/hw/m68k/Makefile.objs
+++ b/hw/m68k/Makefile.objs
@@ -1,2 +1,4 @@
-obj-y += an5206.o mcf5208.o
-obj-y += mcf5206.o mcf_intc.o
+obj-$(CONFIG_COLDFIRE) += an5206.o mcf5208.o
+obj-$(CONFIG_MAC) += mac.o
+
+obj-$(CONFIG_COLDFIRE) += mcf5206.o mcf_intc.o
diff --git a/hw/m68k/bootinfo.h b/hw/m68k/bootinfo.h
new file mode 100644
index 00..b04153ce1e
--- /dev/null
+++ b/hw/m68k/bootinfo.h
@@ -0,0 +1,99 @@
+struct bi_record {
+uint16_t tag;/* tag ID */
+uint16_t size;   /* size of record */
+uint32_t data[0];/* data */
+};
+
+/* machine independent tags */
+
+#define BI_LAST 0x /* last record */
+#define BI_MACHTYPE 0x0001 /* machine type (u_long) */
+#define BI_CPUTYPE  0x0002 /* cpu type (u_long) */
+#define BI_FPUTYPE  0x0003 /* fpu type (u_long) */
+#define BI_MMUTYPE  0x0004 /* mmu type (u_long) */
+#define BI_MEMCHUNK 0x0005 /* memory chunk address and size */
+   /* (struct mem_info) */
+#define BI_RAMDISK  0x0006 /* ramdisk address and size */
+   /* (struct mem_info) */
+#define BI_COMMAND_LINE 0x0007 /* kernel command line parameters */
+   /* (string) */
+
+/*  Macintosh-specific tags (all u_long) */
+
+#define BI_MAC_MODEL0x8000  

Re: [Qemu-block] [Qemu-devel] [RFC 06/13] ESP: add pseudo-DMA as used by Macintosh

2018-06-09 Thread Mark Cave-Ayland

On 08/06/18 21:05, Laurent Vivier wrote:


From: Laurent Vivier 

Signed-off-by: Laurent Vivier 
---
  hw/mips/mips_jazz.c   |   2 +-
  hw/scsi/esp.c | 330 +-
  include/hw/scsi/esp.h |  15 ++-
  3 files changed, 313 insertions(+), 34 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 90cb306f53..87118f2d03 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -282,7 +282,7 @@ static void mips_jazz_init(MachineState *machine,
  
  /* SCSI adapter */

  esp = esp_init(0x80002000, 0, rc4030_dma_read, rc4030_dma_write, dmas[0],
-   qdev_get_gpio_in(rc4030, 5), _reset, _enable);
+   qdev_get_gpio_in(rc4030, 5), NULL, _reset, _enable);
  scsi_bus_legacy_handle_cmdline(>bus);
  
  /* Floppy */


A quick note here: last year I removed esp_init() from all of the SPARC 
code which means that mips/jazz is the last remaining user.


If we can switch this over to using qdev properties similar to how I did 
in 7f773ff5d0: sparc32_dma: make esp device child of espdma device then 
we can remove this function completely.


Hervé, is this something that you could take a quick look at?


ATB,

Mark.



Re: [Qemu-block] [Qemu-devel] [RFC 04/13] hw/m68k: add video card

2018-06-09 Thread Mark Cave-Ayland

On 08/06/18 21:05, Laurent Vivier wrote:


From: Laurent Vivier 

Signed-off-by: Laurent Vivier 
---
  arch_init.c |   4 +
  hw/display/Makefile.objs|   1 +
  hw/display/macfb-template.h | 158 +
  hw/display/macfb.c  | 283 
  qemu-options.hx |   2 +-
  vl.c|   3 +-
  6 files changed, 449 insertions(+), 2 deletions(-)
  create mode 100644 hw/display/macfb-template.h
  create mode 100644 hw/display/macfb.c

diff --git a/arch_init.c b/arch_init.c
index f4f3f610c8..5a71b48dc5 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -39,6 +39,10 @@
  int graphic_width = 1024;
  int graphic_height = 768;
  int graphic_depth = 8;
+#elif defined(TARGET_M68K)
+int graphic_width = 800;
+int graphic_height = 600;
+int graphic_depth = 8;
  #else
  int graphic_width = 800;
  int graphic_height = 600;
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index b5d97ab26d..925d5b848f 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -19,6 +19,7 @@ common-obj-$(CONFIG_EXYNOS4) += exynos4210_fimd.o
  common-obj-$(CONFIG_FRAMEBUFFER) += framebuffer.o
  common-obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o
  common-obj-$(CONFIG_ZAURUS) += tc6393xb.o
+common-obj-$(CONFIG_MACFB) += macfb.o
  
  common-obj-$(CONFIG_MILKYMIST_TMU2) += milkymist-tmu2.o

  milkymist-tmu2.o-cflags := $(X11_CFLAGS)
diff --git a/hw/display/macfb-template.h b/hw/display/macfb-template.h
new file mode 100644
index 00..b6ae5d728f
--- /dev/null
+++ b/hw/display/macfb-template.h
@@ -0,0 +1,158 @@
+#if defined(READ_BITS)
+#define PALETTE(i, r, g, b)\
+do {   \
+r =  s->color_palette[i * 3];  \
+g =  s->color_palette[i * 3 + 1];  \
+b =  s->color_palette[i * 3 + 2];  \
+} while (0)
+
+#if READ_BITS == 1
+#define READ_PIXEL(from, x, r, g, b)   \
+do {   \
+int bit = x & 7;   \
+int idx = (*from >> (7 - bit)) & 1;\
+r = g = b  = ((1 - idx) << 7); \
+from += (bit == 7);\
+} while (0)
+#elif READ_BITS == 2
+#define READ_PIXEL(from, x, r, g, b)   \
+do {   \
+int bit = (x & 3); \
+int idx = (*from >> ((3 - bit) << 1)) & 3; \
+PALETTE(idx, r, g, b); \
+from += (bit == 3);\
+} while (0)
+#elif READ_BITS == 4
+#define READ_PIXEL(from, x, r, g, b)   \
+do {   \
+int bit = x & 1;   \
+int idx = (*from >> ((1 - bit) << 2)) & 15; \
+PALETTE(idx, r, g, b); \
+from += (bit == 1);\
+} while (0)
+#elif READ_BITS == 8
+#define READ_PIXEL(from, x, r, g, b)   \
+do {   \
+PALETTE(*from, r, g, b);   \
+from++;\
+} while (0)
+#elif READ_BITS == 16
+#define READ_PIXEL(from, x, r, g, b)   \
+do {   \
+uint16_t pixel;\
+pixel = (from[0] << 8) | from[1];  \
+r = ((pixel >> 10) & 0x1f) << 3;   \
+g = ((pixel >> 5) & 0x1f) << 3;\
+b = (pixel & 0x1f) << 3;   \
+from += 2; \
+} while (0)
+#elif READ_BITS == 24
+#define READ_PIXEL(from, x, r, g, b)   \
+do {   \
+r = *from++;   \
+g = *from++;   \
+b = *from++;   \
+} while (0)
+#else
+#error unknown bit depth
+#endif
+
+#if WRITE_BITS == 8
+#define WRITE_PIXEL(to, r, g, b)   \
+do {   \
+*to = rgb_to_pixel8(r, g, b);  \
+to += 1;   \
+} while (0)
+#elif WRITE_BITS == 15
+#define WRITE_PIXEL(to, r, g, b)   \
+do {   \
+*(uint16_t *)to = rgb_to_pixel15(r, g, b); \
+to += 2;   \
+} while (0)
+#elif WRITE_BITS == 16
+#define WRITE_PIXEL(to, r, g, b)   \
+do {   \
+*(uint16_t *)to = rgb_to_pixel16(r, g, b); \
+to += 2;   \
+} while (0)
+#elif WRITE_BITS == 24
+#define WRITE_PIXEL(to, r, g, b)   \
+do {  

Re: [Qemu-block] [Qemu-devel] [RFC 03/13] escc: introduce a selector for the register bit

2018-06-09 Thread Mark Cave-Ayland

On 08/06/18 21:05, Laurent Vivier wrote:


From: Laurent Vivier 

On Sparc and PowerMac, the bit 0 of the address
selects the register type (control or data) and
bit 1 selects the channel (B or A).

On m68k Macintosh, the bit 0 selects the channel and
bit 1 the register type.

This patch introduces a new parameter (bit_swap) to
the device interface to indicate bits usage must
be swapped between registers and channels.

For the moment all the machines use the bit 0,
but this change will be needed to emulate Quadra 800.

Signed-off-by: Laurent Vivier 
---
  hw/char/escc.c | 30 --
  include/hw/char/escc.h |  1 +
  2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 628f5f81f7..cec75b06f9 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -42,14 +42,21 @@
   * mouse and keyboard ports don't implement all functions and they are
   * only asynchronous. There is no DMA.
   *
- * Z85C30 is also used on PowerMacs. There are some small differences
- * between Sparc version (sunzilog) and PowerMac (pmac):
+ * Z85C30 is also used on PowerMacs and m68k Macs.
+ *
+ * There are some small differences between Sparc version (sunzilog)
+ * and PowerMac (pmac):
   *  Offset between control and data registers
   *  There is some kind of lockup bug, but we can ignore it
   *  CTS is inverted
   *  DMA on pmac using DBDMA chip
   *  pmac can do IRDA and faster rates, sunzilog can only do 38400
   *  pmac baud rate generator clock is 3.6864 MHz, sunzilog 4.9152 MHz
+ *
+ * Linux driver for m68k Macs is the same as for PowerMac (pmac_zilog),
+ * but registers are grouped by type and not by channel:
+ * channel is selected by bit 0 of the address (instead of bit 1)
+ * and register is selected by bit 1 of the address (instead of bit 0).
   */
  
  /*

@@ -169,6 +176,16 @@ static void handle_kbd_command(ESCCChannelState *s, int 
val);
  static int serial_can_receive(void *opaque);
  static void serial_receive_byte(ESCCChannelState *s, int ch);
  
+static int reg_shift(ESCCState *s)

+{
+return s->bit_swap ? s->it_shift + 1 : s->it_shift;
+}
+
+static int chn_shift(ESCCState *s)
+{
+return s->bit_swap ? s->it_shift : s->it_shift + 1;
+}
+
  static void clear_queue(void *opaque)
  {
  ESCCChannelState *s = opaque;
@@ -433,8 +450,8 @@ static void escc_mem_write(void *opaque, hwaddr addr,
  int newreg, channel;
  
  val &= 0xff;

-saddr = (addr >> serial->it_shift) & 1;
-channel = (addr >> (serial->it_shift + 1)) & 1;
+saddr = (addr >> reg_shift(serial)) & 1;
+channel = (addr >> chn_shift(serial)) & 1;
  s = >chn[channel];
  switch (saddr) {
  case SERIAL_CTRL:
@@ -537,8 +554,8 @@ static uint64_t escc_mem_read(void *opaque, hwaddr addr,
  uint32_t ret;
  int channel;
  
-saddr = (addr >> serial->it_shift) & 1;

-channel = (addr >> (serial->it_shift + 1)) & 1;
+saddr = (addr >> reg_shift(serial)) & 1;
+channel = (addr >> chn_shift(serial)) & 1;
  s = >chn[channel];
  switch (saddr) {
  case SERIAL_CTRL:
@@ -822,6 +839,7 @@ static void escc_realize(DeviceState *dev, Error **errp)
  static Property escc_properties[] = {
  DEFINE_PROP_UINT32("frequency", ESCCState, frequency,   0),
  DEFINE_PROP_UINT32("it_shift",  ESCCState, it_shift,0),
+DEFINE_PROP_BOOL("bit_swap",ESCCState, bit_swap,false),
  DEFINE_PROP_UINT32("disabled",  ESCCState, disabled,0),
  DEFINE_PROP_UINT32("chnBtype",  ESCCState, chn[0].type, 0),
  DEFINE_PROP_UINT32("chnAtype",  ESCCState, chn[1].type, 0),
diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
index 42aca83611..8762f61c14 100644
--- a/include/hw/char/escc.h
+++ b/include/hw/char/escc.h
@@ -50,6 +50,7 @@ typedef struct ESCCState {
  
  struct ESCCChannelState chn[2];

  uint32_t it_shift;
+bool bit_swap;
  MemoryRegion mmio;
  uint32_t disabled;
  uint32_t frequency;



I think that this is basically okay, but "bit_swap" doesn't really tell 
me much about what is being swapped - maybe something along the lines of 
"channel-bit0-reg-bit1"?



ATB,

Mark.



Re: [Qemu-block] [Qemu-devel] [RFC 01/13] hw/m68k: add via support

2018-06-09 Thread Mark Cave-Ayland

On 08/06/18 21:05, Laurent Vivier wrote:


Signed-off-by: Laurent Vivier 
---
  hw/input/adb.c|  99 -
  hw/misc/Makefile.objs |   1 +
  hw/misc/mac_via.c | 940 ++
  include/hw/input/adb.h|   8 +
  include/hw/misc/mac_via.h |  45 +++
  5 files changed, 1092 insertions(+), 1 deletion(-)
  create mode 100644 hw/misc/mac_via.c
  create mode 100644 include/hw/misc/mac_via.h

diff --git a/hw/input/adb.c b/hw/input/adb.c
index 23ae6f0d75..2e5460730c 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -25,6 +25,17 @@
  #include "hw/input/adb.h"
  #include "adb-internal.h"
  
+#define ADB_POLL_FREQ 50

+
+/* Apple Macintosh Family Hardware Refenece
+ * Table 19-10 ADB transaction states
+ */
+
+#define STATE_NEW   0
+#define STATE_EVEN  1
+#define STATE_ODD   2
+#define STATE_IDLE  3
+
  /* error codes */
  #define ADB_RET_NOTPRESENT (-2)
  
@@ -57,7 +68,6 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len)

  return ADB_RET_NOTPRESENT;
  }
  
-/* XXX: move that to cuda ? */

  int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
  {
  ADBDevice *d;
@@ -84,6 +94,93 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t 
poll_mask)
  return olen;
  }
  
+int adb_send(ADBBusState *adb, int state, uint8_t data)

+{
+switch (state) {
+case STATE_NEW:
+adb->data_out[0] = data;
+adb->data_out_index = 1;
+break;
+case STATE_EVEN:
+if ((adb->data_out_index & 1) == 0) {
+return 0;
+}
+adb->data_out[adb->data_out_index++] = data;
+break;
+case STATE_ODD:
+if (adb->data_out_index & 1) {
+return 0;
+}
+adb->data_out[adb->data_out_index++] = data;
+break;
+case STATE_IDLE:
+return 0;
+}
+qemu_irq_raise(adb->data_ready);
+return 1;
+}
+
+int adb_receive(ADBBusState *adb, int state, uint8_t *data)
+{
+switch (state) {
+case STATE_NEW:
+return 0;
+case STATE_EVEN:
+if (adb->data_in_size <= 0) {
+qemu_irq_raise(adb->data_ready);
+return 0;
+}
+if (adb->data_in_index >= adb->data_in_size) {
+*data = 0;
+qemu_irq_raise(adb->data_ready);
+return 1;
+}
+if ((adb->data_in_index & 1) == 0) {
+return 0;
+}
+*data = adb->data_in[adb->data_in_index++];
+break;
+case STATE_ODD:
+if (adb->data_in_size <= 0) {
+qemu_irq_raise(adb->data_ready);
+return 0;
+}
+if (adb->data_in_index >= adb->data_in_size) {
+*data = 0;
+qemu_irq_raise(adb->data_ready);
+return 1;
+}
+if (adb->data_in_index & 1) {
+return 0;
+}
+*data = adb->data_in[adb->data_in_index++];
+break;
+case STATE_IDLE:
+if (adb->data_out_index == 0) {
+return 0;
+}
+adb->data_in_size = adb_request(adb, adb->data_in,
+adb->data_out, adb->data_out_index);
+adb->data_out_index = 0;
+if (adb->data_in_size < 0) {
+*data = 0xff;
+qemu_irq_raise(adb->data_ready);
+return -1;
+}
+if (adb->data_in_size == 0) {
+return 0;
+}
+*data = adb->data_in[0];
+adb->data_in_index = 1;
+break;
+}
+qemu_irq_raise(adb->data_ready);
+if (*data == 0xff || *data == 0) {
+return 0;
+}
+return 1;
+}
+
  static const TypeInfo adb_bus_type_info = {
  .name = TYPE_ADB_BUS,
  .parent = TYPE_BUS,


I'm not completely convinced by this part: from working on Mac PMU 
patches my feeling is that any buffering should be handled by the level 
above the ADB bus, since once you've sent/received something the 
supporting logic appears completely different.


Having said that, the ADB bus state part is new so I'm not sure I 
understand why this is currently needed?



diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 00e834d0f0..2cd8941faa 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -68,5 +68,6 @@ obj-$(CONFIG_PVPANIC) += pvpanic.o
  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
  obj-$(CONFIG_AUX) += auxbus.o
  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-$(CONFIG_MAC_VIA) += mac_via.o
  obj-y += mmio_interface.o
  obj-$(CONFIG_MSF2) += msf2-sysreg.o
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
new file mode 100644
index 00..a6a11c5b3d
--- /dev/null
+++ b/hw/misc/mac_via.c
@@ -0,0 +1,940 @@
+/*
+ * QEMU m68k Macintosh VIA device support
+ *
+ * Copyright (c) 2011-2018 Laurent Vivier
+ *
+ * Some parts from hw/cuda.c
+ *
+ * Copyright (c) 2004-2007 Fabrice Bellard
+ * Copyright (c) 2007 Jocelyn Mayer
+ *
+ * some parts from linux-2.6.29, 

Re: [Qemu-block] [RFC 06/13] ESP: add pseudo-DMA as used by Macintosh

2018-06-09 Thread Hervé Poussineau

Le 08/06/2018 à 22:05, Laurent Vivier a écrit :

From: Laurent Vivier 

Signed-off-by: Laurent Vivier 
---
  hw/mips/mips_jazz.c   |   2 +-
  hw/scsi/esp.c | 330 +-
  include/hw/scsi/esp.h |  15 ++-
  3 files changed, 313 insertions(+), 34 deletions(-)


Works OK on NetBSD 5.1/arc on MIPS Magnum.

Tested-by: Hervé Poussineau 




Re: [Qemu-block] [RFC 12/13] dp8393x: put DMA temp buffer in the state, not in the stack

2018-06-09 Thread Hervé Poussineau

Le 08/06/2018 à 22:05, Laurent Vivier a écrit :

It's only 32 bytes, and this simplifies the dp8393x_get()/
dp8393x_put() interface.

Signed-off-by: Laurent Vivier 
---
  hw/net/dp8393x.c | 107 ++-
  1 file changed, 51 insertions(+), 56 deletions(-)



Works OK on NetBSD 5.1/arc on MIPS Magnum.

Tested-by: Hervé Poussineau 






Re: [Qemu-block] [RFC 11/13] dp8393x: manage big endian bus

2018-06-09 Thread Hervé Poussineau

Le 08/06/2018 à 22:05, Laurent Vivier a écrit :

This is needed by Quadra 800, this card can run on little-endian
or big-endian bus.

Signed-off-by: Laurent Vivier 
---
  hw/net/dp8393x.c | 101 ++-
  1 file changed, 70 insertions(+), 31 deletions(-)



Works OK on NetBSD 5.1/arc on MIPS Magnum.

Tested-by: Hervé Poussineau 






Re: [Qemu-block] [RFC 13/13] dp8393x: fix receiving buffer exhaustion

2018-06-09 Thread Hervé Poussineau

Le 08/06/2018 à 22:05, Laurent Vivier a écrit :

The card is not able to exit from exhaustion state, because
while the drive consumes the buffers, the RRP is incremented
(when the driver clears the ISR RBE bit), so it stays equal
to RWP, and while RRP == RWP, the card thinks it is always
in exhaustion state. So the driver consumes all the buffers,
but the card cannot receive new ones.

This patch fixes the problem by not incrementing RRP when
the driver clears the ISR RBE bit.

Signed-off-by: Laurent Vivier 
---
  hw/net/dp8393x.c | 31 ---
  1 file changed, 16 insertions(+), 15 deletions(-)



Works OK on NetBSD 5.1/arc on MIPS Magnum.

Tested-by: Hervé Poussineau 




Re: [Qemu-block] [RFC 10/13] dp8393x: fix dp8393x_receive

2018-06-09 Thread Hervé Poussineau

Le 08/06/2018 à 22:05, Laurent Vivier a écrit :

address_space_rw() access size must be multiplied by width.
dp8393x_receive() must return the number of bytes read, not the length
of the last memory access.

Signed-off-by: Laurent Vivier 
---
  hw/net/dp8393x.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index f2d2ce344c..ef5f1eb94f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -808,9 +808,11 @@ static ssize_t dp8393x_receive(NetClientState *nc, const 
uint8_t * buf,
  /* EOL detected */
  s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
  } else {
+size = sizeof(uint16_t) * width;
  data[0 * width] = 0; /* in_use */
-address_space_rw(>as, dp8393x_crda(s) + sizeof(uint16_t) * 6 * 
width,
-MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, sizeof(uint16_t), 1);
+address_space_rw(>as,
+dp8393x_crda(s) + sizeof(uint16_t) * 6 * width,
+MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
  s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
  s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
  s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] 
& 0x00ff) + 1) & 0x00ff);
@@ -824,7 +826,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const 
uint8_t * buf,
  /* Done */
  dp8393x_update_irq(s);
  
-return size;

+return rx_len;
  }
  
  static void dp8393x_reset(DeviceState *dev)




NACK. This breaks NetBSD 5.1/arc networking.

This seems to revert the following commit and change the function return value.

commit 409b52bfe199d8106dadf7c5ff3d88d2228e89b5
Author: Hervé Poussineau 
Date:   Wed Jun 3 22:45:48 2015 +0200

net/dp8393x: correctly reset in_use field

Don't write more than the field width, which is always 16 bit.
Fixes network in NetBSD 5.1/arc

Signed-off-by: Hervé Poussineau 
Reviewed-by: Aurelien Jarno 
Signed-off-by: Leon Alrae 

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 4184045145..ff633f76a0 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -764,7 +764,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const 
uint8_t * buf,
 data[0 * width] = 0; /* in_use */
 address_space_rw(>as,
 ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + 
sizeof(uint16_t) * 6 * width,
-MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
+MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, sizeof(uint16_t), 1);
 s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
 s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
 s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] 
& 0x00ff) + 1) & 0x00ff);




Re: [Qemu-block] [Qemu-devel] [RFC 00/13] hw/m68k: add Apple Machintosh Quadra 800 machine

2018-06-09 Thread Laurent Vivier
Le 09/06/2018 à 05:26, Philippe Mathieu-Daudé a écrit :
> On 06/08/2018 05:05 PM, Laurent Vivier wrote:
>> I'm rebasing some of these patches for seven years now,
>> too many years...
>>
>> It's an RFC because things have changed in QEMU in seven years,
>> for instance the VIA has a new implementation (mos6522) introduced
>> by Mark Cave-Ayland and I didn't rework my implementation to
>> fit into this new one (any volunteers?), display has some glitches,
>> ADB devices are not identified correctly.
>>
>> if you want to test the machine, I'm sorry, it doesn't boot
>> a MacROM, but you can boot a linux kernel from the command line.
>>
>> You can install your own disk using debian-installer, with:
>>
>> ...
>> -M q800 \
>> -serial none -serial mon:stdio \
>> -m 1000M -drive file=m68k.qcow2,format=qcow2 \
>> -net nic,model=dp83932,addr=09:00:07:12:34:57 \
>> -append "console=ttyS0 vga=off" \
>> -kernel vmlinux-4.15.0-2-m68k \
>> -initrd initrd.gz \
>> -drive file=debian-9.0-m68k-NETINST-1.iso \
>> -drive file=m68k.qcow2,format=qcow2 \
>> -nographic
> 
> qemu-system-m68k: -drive file=m68k.qcow2,format=qcow2: Failed to get
> "write" lock
> Is another process using the image?
> 
> (two times same file provided in cmdline arguments)

Yes, you're right. A bad cut'n'paste.

Thanks,
Laurent




Re: [Qemu-block] [Qemu-devel] [PATCH v4 0/6] NBD export

2018-06-09 Thread no-reply
Hi,

This series failed build test on s390x host. Please find the details below.

Type: series
Message-id: 20180608152353.98712-1-vsement...@virtuozzo.com
Subject: [Qemu-devel] [PATCH v4 0/6] NBD export

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20180608190145.10872-1-f4...@amsat.org -> 
patchew/20180608190145.10872-1-f4...@amsat.org
 * [new tag]   patchew/20180608192940.19548-1-ehabk...@redhat.com 
-> patchew/20180608192940.19548-1-ehabk...@redhat.com
Switched to a new branch 'test'
9f22d89e98 docs/interop: add nbd.txt
0a7486a3ae qapi: new qmp command nbd-server-add-bitmap
b0ac84b12c nbd/server: implement dirty bitmap export
13b0fec9fa nbd/server: add nbd_meta_empty_or_pattern helper
f40478968c nbd/server: refactor NBDExportMetaContexts
988893b74c nbd/server: fix trace

=== OUTPUT BEGIN ===
=== ENV ===
LANG=en_US.UTF-8
XDG_SESSION_ID=216587
USER=fam
PWD=/var/tmp/patchew-tester-tmp-c4_c2p9m/src
HOME=/home/fam
SHELL=/bin/sh
SHLVL=2
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
PATH=/usr/bin:/bin
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
glibc-debuginfo-common-2.24-10.fc25.s390x
fedora-release-26-1.noarch
dejavu-sans-mono-fonts-2.35-4.fc26.noarch
xemacs-filesystem-21.5.34-22.20170124hgf412e9f093d4.fc26.noarch
bash-4.4.12-7.fc26.s390x
libSM-1.2.2-5.fc26.s390x
libmpc-1.0.2-6.fc26.s390x
libaio-0.3.110-7.fc26.s390x
libverto-0.2.6-7.fc26.s390x
perl-Scalar-List-Utils-1.48-1.fc26.s390x
iptables-libs-1.6.1-2.fc26.s390x
tcl-8.6.6-2.fc26.s390x
libxshmfence-1.2-4.fc26.s390x
expect-5.45-23.fc26.s390x
perl-Thread-Queue-3.12-1.fc26.noarch
perl-encoding-2.19-6.fc26.s390x
keyutils-1.5.10-1.fc26.s390x
gmp-devel-6.1.2-4.fc26.s390x
enchant-1.6.0-16.fc26.s390x
python-gobject-base-3.24.1-1.fc26.s390x
python3-enchant-1.6.10-1.fc26.noarch
python-lockfile-0.11.0-6.fc26.noarch
python2-pyparsing-2.1.10-3.fc26.noarch
python2-lxml-4.1.1-1.fc26.s390x
librados2-10.2.7-2.fc26.s390x
trousers-lib-0.3.13-7.fc26.s390x
libdatrie-0.2.9-4.fc26.s390x
libsoup-2.58.2-1.fc26.s390x
passwd-0.79-9.fc26.s390x
bind99-libs-9.9.10-3.P3.fc26.s390x
python3-rpm-4.13.0.2-1.fc26.s390x
systemd-233-7.fc26.s390x
virglrenderer-0.6.0-1.20170210git76b3da97b.fc26.s390x
s390utils-ziomon-1.36.1-3.fc26.s390x
s390utils-osasnmpd-1.36.1-3.fc26.s390x
libXrandr-1.5.1-2.fc26.s390x
libglvnd-glx-1.0.0-1.fc26.s390x
texlive-ifxetex-svn19685.0.5-33.fc26.2.noarch
texlive-psnfss-svn33946.9.2a-33.fc26.2.noarch
texlive-dvipdfmx-def-svn40328-33.fc26.2.noarch
texlive-natbib-svn20668.8.31b-33.fc26.2.noarch
texlive-xdvi-bin-svn40750-33.20160520.fc26.2.s390x
texlive-cm-svn32865.0-33.fc26.2.noarch
texlive-beton-svn15878.0-33.fc26.2.noarch
texlive-fpl-svn15878.1.002-33.fc26.2.noarch
texlive-mflogo-svn38628-33.fc26.2.noarch
texlive-texlive-docindex-svn41430-33.fc26.2.noarch
texlive-luaotfload-bin-svn34647.0-33.20160520.fc26.2.noarch
texlive-koma-script-svn41508-33.fc26.2.noarch
texlive-pst-tree-svn24142.1.12-33.fc26.2.noarch
texlive-breqn-svn38099.0.98d-33.fc26.2.noarch
texlive-xetex-svn41438-33.fc26.2.noarch
gstreamer1-plugins-bad-free-1.12.3-1.fc26.s390x
xorg-x11-font-utils-7.5-33.fc26.s390x
ghostscript-fonts-5.50-36.fc26.noarch
libXext-devel-1.3.3-5.fc26.s390x
libusbx-devel-1.0.21-2.fc26.s390x
libglvnd-devel-1.0.0-1.fc26.s390x
emacs-25.3-3.fc26.s390x
alsa-lib-devel-1.1.4.1-1.fc26.s390x
kbd-2.0.4-2.fc26.s390x
dconf-0.26.0-2.fc26.s390x
mc-4.8.19-5.fc26.s390x
doxygen-1.8.13-9.fc26.s390x
dpkg-1.18.24-1.fc26.s390x
libtdb-1.3.13-1.fc26.s390x
python2-pynacl-1.1.1-1.fc26.s390x
perl-Filter-1.58-1.fc26.s390x
python2-pip-9.0.1-11.fc26.noarch
dnf-2.7.5-2.fc26.noarch
bind-license-9.11.2-1.P1.fc26.noarch
libtasn1-4.13-1.fc26.s390x
cpp-7.3.1-2.fc26.s390x
pkgconf-1.3.12-2.fc26.s390x
python2-fedora-0.10.0-1.fc26.noarch
cmake-filesystem-3.10.1-11.fc26.s390x
python3-requests-kerberos-0.12.0-1.fc26.noarch
libmicrohttpd-0.9.59-1.fc26.s390x
GeoIP-GeoLite-data-2018.01-1.fc26.noarch
python2-libs-2.7.14-7.fc26.s390x
libidn2-2.0.4-3.fc26.s390x
p11-kit-devel-0.23.10-1.fc26.s390x
perl-Errno-1.25-396.fc26.s390x
libdrm-2.4.90-2.fc26.s390x
sssd-common-1.16.1-1.fc26.s390x
boost-random-1.63.0-11.fc26.s390x
urw-fonts-2.4-24.fc26.noarch
ccache-3.3.6-1.fc26.s390x
glibc-debuginfo-2.24-10.fc25.s390x
dejavu-fonts-common-2.35-4.fc26.noarch