Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling

2024-06-04 Thread Dr. David Alan Gilbert
provide good support and work together. > > > > > > > > > We are happy to provide the necessary review and maintenance work for > > > > RDMA > > > > if the community needs it. > > > > > > > > CC'ing Chuan Zheng.

Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling

2024-06-05 Thread Dr. David Alan Gilbert
* Peter Xu (pet...@redhat.com) wrote: > Hey, Dave! Hey! > On Wed, Jun 05, 2024 at 12:31:56AM +0000, Dr. David Alan Gilbert wrote: > > * Michael Galaxy (mgal...@akamai.com) wrote: > > > One thing to keep in mind here (despite me not having any hardware to > > > t

Re: [Qemu-block] [PATCH v5 1/4] Add new block driver interface to add/delete a BDS's child

2015-10-07 Thread Dr. David Alan Gilbert
2f2c47b..64cbc55 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -288,6 +288,11 @@ struct BlockDriver { > */ > int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo); > > +void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child, > + Error **errp); > +void (*bdrv_del_child)(BlockDriverState *parent, BlockDriverState *child, > + Error **errp); > + > QLIST_ENTRY(BlockDriver) list; > }; > > -- > 2.4.3 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH v5 1/4] Add new block driver interface to add/delete a BDS's child

2015-10-08 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 10/08/2015 03:00 AM, Dr. David Alan Gilbert wrote: > > * Wen Congyang (we...@cn.fujitsu.com) wrote: > >> In some cases, we want to take a quorum child offline, and take > >> another child online. > > > > H

Re: [Qemu-block] [PATCH v2] migration: disallow migrate_add_blocker during migration

2015-10-09 Thread Dr. David Alan Gilbert
on to prohibit e.g. block jobs > from running concurrently with migration. > > Signed-off-by: John Snow For the migration code only: Reviewed-by: Dr. David Alan Gilbert Dave > --- > block/qcow.c | 6 +- > block/vdi.c | 6 +- &

Re: [Qemu-block] [Qemu-devel] [PATCH v5 3/4] qmp: add monitor command to add/remove a child

2015-10-09 Thread Dr. David Alan Gilbert
t; so much. As I argued above, I'm not opposed to adding interface that are > actually not what we want, but that are what users want, and that are > easy to implement with the interface that we want. It's what I call a > “macro function”. > > > A secondary question is whether the incompleteness of the implementation > > demands an x- to warn users. > > We don't want to shoehorn generality into these two functions, but add a > new one anyway. We might want to add optional parameters later on (e.g. > changing the threshold), but that would be a compatible change. > > >> (My point is that with such an experimental interface, management tools > >> cannot use it, even though it'd be a very nice functionality to have) > > > > How much work is it to finish the job? Unless it's a substantial chunk, > > debating x- is much ado about nothing: just finish the job already :) > > We have proven in the past to need a significant amount of time for even > for non-substantial chunks. Often, non-substantial chunks turn out to be > substantial when actually tackling them. > > It basically comes down to whether the COLO authors are willing to wait > for us to do the job. And frankly, if I were them, I wouldn't be. > > Max > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH v5 3/4] qmp: add monitor command to add/remove a child

2015-10-12 Thread Dr. David Alan Gilbert
* Max Reitz (mre...@redhat.com) wrote: > On 09.10.2015 18:42, Dr. David Alan Gilbert wrote: > > * Max Reitz (mre...@redhat.com) wrote: > >> On 08.10.2015 08:15, Markus Armbruster wrote: > >>> Max Reitz writes: > >>> > >>>> On 22.09.2015 09

Re: [Qemu-block] [Qemu-devel] [PATCH 00/17] Framework for securely passing secrets to QEMU

2015-10-19 Thread Dr. David Alan Gilbert
> tests/.gitignore| 1 + > tests/Makefile | 2 + > tests/qemu-iotests/087 | 20 + > tests/qemu-iotests/087.out | 26 +- > tests/qemu-iotests/134 | 17 +- > tests/qemu-iotests/134.out | 44 +-- > tests/qemu-iotests/common.rc| 4 +- > tests/test-crypto-secret.c | 440 ++ > util/oslib-posix.c | 66 > util/oslib-win32.c | 24 -- > util/qemu-option.c | 6 + > vl.c| 8 +- > 45 files changed, 2740 insertions(+), 751 deletions(-) > create mode 100644 crypto/secret.c > create mode 100644 include/crypto/secret.h > create mode 100644 tests/test-crypto-secret.c > > -- > 2.4.3 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Patch v12 00/10] Block replication for continuous checkpoints

2015-12-01 Thread Dr. David Alan Gilbert
> blockjob.c | 11 + > docs/block-replication.txt | 227 +++ > include/block/block.h | 9 + > include/block/block_int.h | 15 ++ > include/block/blockjob.h | 12 + > qapi/block-core.json | 34 ++- > 11 files changed, 1093 insertions(+), 4 deletions(-) > create mode 100644 block/replication.c > create mode 100644 docs/block-replication.txt > > -- > 2.5.0 > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] HMP: Add equivalent to x-blockdev-change

2015-12-17 Thread Dr. David Alan Gilbert
* Eric Blake (ebl...@redhat.com) wrote: > On 12/17/2015 03:47 AM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" > > > > x-blockdev-change has no HMP equivalent, so add x_block_change. > > > > Example useages are: > > s/

Re: [Qemu-block] [Patch v12 resend 05/10] docs: block replication's description

2016-01-04 Thread Dr. David Alan Gilbert
to take some time (seconds?) to sync Host 3 back in when you add it after a failover and the aim would be not to have distrubed the application for that long, so it should already be running on Host 2 during that resync. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Patch v12 resend 00/10] Block replication for continuous checkpoints

2016-01-04 Thread Dr. David Alan Gilbert
onState *brs); > > void block_replication_remove(BlockReplicationState *brs); > > > > The replication block driver would add/remove itself. The quorum block > > driver probably doesn't need to be modified (I think in your current > > patches you modify it just to forward the start/stop/checkpoint calls to > > a particular quorum child). > > Yes, it is the major purpose. We also do some check in the quorum driver: > we don't allow more than one child support block replication. > > Thanks > Wen Congyang > > > > > Stefan > > > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] HMP: Add equivalent to x-blockdev-change

2016-01-18 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert (git)" writes: > > > From: "Dr. David Alan Gilbert" > > > > x-blockdev-change has no HMP equivalent, so add x_block_change. > > Uh, I can find neither QMP command x-block

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-01-22 Thread Dr. David Alan Gilbert
cb=0x57815410) at /root/colo/jan-2016/qemu/block/io.c:2122 . So I guess acb->child_iter is wrong, since we only have one child on that quorum? and we're trying to do a destroy on the second child. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-block] COLO: how to flip a secondary to a primary?

2016-01-22 Thread Dr. David Alan Gilbert
existing connections need to keep their sequence number offset but new connections made by what is now the primary dont need to do anything special. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

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

2016-01-22 Thread Dr. David Alan Gilbert
gt; > contents as the backing file. I don't think qemu has to protect us > > from user error in this case. > > But the backing file is read-only so the user can guarantee that the > destination has the same data before the shallow mirror. How do you do > that in this case

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-01-25 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 01/22/2016 11:14 PM, Dr. David Alan Gilbert wrote: > > Hi, > > I can trigger a segfault if I wire in the block replication together with > > a quorum instance; it only triggers with both of them present but, > > it

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-01-25 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 01/22/2016 11:14 PM, Dr. David Alan Gilbert wrote: > > Hi, > > I can trigger a segfault if I wire in the block replication together with > > a quorum instance; it only triggers with both of them present but, > > it

Re: [Qemu-block] COLO: how to flip a secondary to a primary?

2016-01-25 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 01/23/2016 03:35 AM, Dr. David Alan Gilbert wrote: > > Hi, > > I've been looking at what's needed to add a new secondary after > > a primary failed; from the block side it doesn't look as hard > > as I&

Re: [Qemu-block] COLO: how to flip a secondary to a primary?

2016-01-25 Thread Dr. David Alan Gilbert
the existing connection? Dave > > > >Hailiang or Zhijian can answer this question. > > > Thanks > Li Zhijian > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-01-27 Thread Dr. David Alan Gilbert
ock/backup.c | 14 ++ > block/quorum.c | 78 +++ > block/replication.c| 545 > + > blockjob.c | 11 + > docs/block-replication.txt | 227 +++ > include/block/block.h | 9 + > include/block/block_int.h | 15 ++ > include/block/blockjob.h | 12 + > qapi/block-core.json | 33 ++- > 11 files changed, 1089 insertions(+), 3 deletions(-) > create mode 100644 block/replication.c > create mode 100644 docs/block-replication.txt > > -- > 1.9.3 > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-01-29 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 01/27/2016 07:03 PM, Dr. David Alan Gilbert wrote: > > Hi, > > I've got a block error if I kill the secondary. > > > > Start both primary & secondary > > kill -9 secondary qemu > > x_colo_lost_hear

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-01-29 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 01/29/2016 06:07 PM, Dr. David Alan Gilbert wrote: > > * Wen Congyang (we...@cn.fujitsu.com) wrote: > >> On 01/27/2016 07:03 PM, Dr. David Alan Gilbert wrote: > >>> Hi, > >>> I've got a block error

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-02-01 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 01/29/2016 06:47 PM, Dr. David Alan Gilbert wrote: > > * Wen Congyang (we...@cn.fujitsu.com) wrote: > >> On 01/29/2016 06:07 PM, Dr. David Alan Gilbert wrote: > >>> * Wen Congyang (we...@cn.fujitsu.com) wrote: >

Re: [Qemu-block] [PATCH v13 00/10] Block replication for continuous checkpoints

2016-02-04 Thread Dr. David Alan Gilbert
* Changlong Xie (xiecl.f...@cn.fujitsu.com) wrote: > On 02/01/2016 09:18 AM, Wen Congyang wrote: > >On 01/29/2016 06:47 PM, Dr. David Alan Gilbert wrote: > >>* Wen Congyang (we...@cn.fujitsu.com) wrote: > >>>On 01/29/2016 06:07 PM, Dr. David Alan Gilbert w

Re: [PATCH v2 18/21] migration: remove the QEMUFileOps 'get_buffer' callback

2022-06-27 Thread Dr. David Alan Gilbert
* Daniel P. Berrangé (berra...@redhat.com) wrote: > This directly implements the get_buffer logic using QIOChannel APIs. > > Reviewed-by: Dr. David Alan Gilbert > Signed-off-by: Daniel P. Berrangé Coverity is pointing out a fun deadcode path from this: > diff --git a/migratio

Re: [PATCH 1/2] migration: Assert that migrate_multifd_compression() returns an in-range value

2022-07-21 Thread Dr. David Alan Gilbert
to return is within range. > > Resolves: Coverity CID 1487239, 1487254 > Signed-off-by: Peter Maydell Reviewed-by: Dr. David Alan Gilbert > --- > migration/migration.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/migration/migration.c b/migration/migration.c

Re: [PATCH 2/2] migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long

2022-07-21 Thread Dr. David Alan Gilbert
to bdrv_create_dirty_bitmap that takes a uint32_t ? Dave > #define BDRV_SECTORS_PER_DIRTY_CHUNK (BLK_MIG_BLOCK_SIZE >> BDRV_SECTOR_BITS) > > #define BLK_MIG_FLAG_DEVICE_BLOCK 0x01 > -- > 2.25.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 2/2] migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long

2022-07-21 Thread Dr. David Alan Gilbert
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On Thu, 21 Jul 2022 at 13:07, Dr. David Alan Gilbert > wrote: > > > > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > > When we use BLK_MIG_BLOCK_SIZE in expressions like > > > block_mig_state.

Re: [PATCH 0/2] migration: fix coverity nits

2022-08-02 Thread Dr. David Alan Gilbert
> migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long > > Ping? This series got reviewed but doesn't seem to have > made it into a migration pullreq yet. Queued > thanks > -- PMM > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [RFC v2 02/10] Drop unused static function return values

2022-08-03 Thread Dr. David Alan Gilbert
**errp) > } > } > > -ret = migrate_caps_check(cap_list, head, errp); > +migrate_caps_check(cap_list, head, errp); > > /* It works with head == NULL */ > qapi_free_MigrationCapabilityStatusList(head); > - > -return ret; > } -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 07/51] tests: Avoid using hardcoded /tmp in test cases

2022-08-24 Thread Dr. David Alan Gilbert
-git a/tests/unit/test-image-locking.c b/tests/unit/test-image-locking.c > index ba057bd66c..795c602ff6 100644 > --- a/tests/unit/test-image-locking.c > +++ b/tests/unit/test-image-locking.c > @@ -76,7 +76,7 @@ static void check_locked_bytes(int fd, uint64_t perm_locks, > static void test_image_locking_basic(void) > { > BlockBackend *blk1, *blk2, *blk3; > -char img_path[] = "/tmp/qtest.XX"; > +char *img_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir()); > uint64_t perm, shared_perm; > > int fd = mkstemp(img_path); > @@ -112,12 +112,13 @@ static void test_image_locking_basic(void) > blk_unref(blk3); > close(fd); > unlink(img_path); > +g_free(img_path); > } > > static void test_set_perm_abort(void) > { > BlockBackend *blk1, *blk2; > -char img_path[] = "/tmp/qtest.XX"; > +char *img_path = g_strdup_printf("%s/qtest.XX", g_get_tmp_dir()); > uint64_t perm, shared_perm; > int r; > int fd = mkstemp(img_path); > @@ -140,6 +141,7 @@ static void test_set_perm_abort(void) > check_locked_bytes(fd, perm, ~shared_perm); > blk_unref(blk1); > blk_unref(blk2); > +g_free(img_path); > } > > int main(int argc, char **argv) > diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c > index a05a4628ed..b73d231cd5 100644 > --- a/tests/unit/test-qga.c > +++ b/tests/unit/test-qga.c > @@ -59,7 +59,7 @@ fixture_setup(TestFixture *fixture, gconstpointer data, > gchar **envp) > > fixture->loop = g_main_loop_new(NULL, FALSE); > > -fixture->test_dir = g_strdup("/tmp/qgatest.XX"); > +fixture->test_dir = g_strdup_printf("%s/qgatest.XX", > g_get_tmp_dir()); > g_assert_nonnull(g_mkdtemp(fixture->test_dir)); > > path = g_build_filename(fixture->test_dir, "sock", NULL); > diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c > index 9b1dab2f28..0da6a6157f 100644 > --- a/tests/vhost-user-bridge.c > +++ b/tests/vhost-user-bridge.c > @@ -631,7 +631,7 @@ static void *notifier_thread(void *arg) > static void > vubr_host_notifier_setup(VubrDev *dev) > { > -char template[] = "/tmp/vubr-XX"; > +char *template = g_strdup_printf("%s/vubr-XX", g_get_tmp_dir()); > pthread_t thread; > size_t length; > void *addr; > @@ -640,6 +640,7 @@ vubr_host_notifier_setup(VubrDev *dev) > length = qemu_real_host_page_size() * VHOST_USER_BRIDGE_MAX_QUEUES; > > fd = mkstemp(template); > +g_free(template); > if (fd < 0) { > vubr_die("mkstemp()"); > } > -- > 2.34.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: Question regarding live-migration with drive-mirror

2022-09-28 Thread Dr. David Alan Gilbert
80 > > #9 blk_co_pwritev (blk=0x5595f91db8c0, offset=, > > bytes=, qiov=qiov@entry=0x5595f9030d58, flags=flags@entry=0) > > at ../block/block-backend.c:1391 > > #10 0x5595f8328a59 in mirror_read_complete (ret=0, op=0x5595f9030d50) > > at ../block/mirror.c:260 > > #11 mirror_co_read (opaque=0x5595f9030d50) at ../block/mirror.c:400 > > #12 0x5595f843a39b in coroutine_trampoline (i0=, > > i1=) at ../util/coroutine-ucontext.c:177 > > #13 0x7f33948b8d40 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 > > #14 0x7f324a3ea6e0 in ?? () > > #15 0x in ?? () > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH] nbd/server: Use smarter assert

2022-10-17 Thread Dr. David Alan Gilbert
SIZE. Still, it doesn't hurt to be more explicit in > how we write our assertion that we are aware that no wraparound is > possible. > > Fixes: 93676c88 ("nbd: Don't send oversize strings", v4.2.0) > Reported-by: Dr. David Alan Gilbert > Signed-off-by: Eric Blake

Re: [PATCH v2] nbd/client: Use smarter assert

2022-10-18 Thread Dr. David Alan Gilbert
r inadvertent wraparound is > possible. > > Fixes: 93676c88 ("nbd: Don't send oversize strings", v4.2.0) > Reported-by: Dr. David Alan Gilbert > Signed-off-by: Eric Blake Reviewed-by: Dr. David Alan Gilbert > --- > > v2: update subject line and commit mess

Re: [PATCH 2/2] thread-pool: use ThreadPool from the running thread

2022-10-20 Thread Dr. David Alan Gilbert
ds on behalf of the main loop, which means pool->head could be > > modified (iothread calls thread_pool_submit_aio) while being read by the > > main loop (another worker thread schedules thread_pool_completion_bh). > > > > What are the performance implications? I mean, if the aiocontext lock in > > the bh is actually useful and the bh really has to wait to take it, > > being taken in much more places throughout the block layer won't be > > better than extending the poll->lock I guess. > > thread_pool_submit_aio() is missing documentation on how it is supposed > to be called. > > Taking pool->lock is conservative and fine in the short-term. > > In the longer term we need to clarify how thread_pool_submit_aio() is > supposed to be used and remove locking to protect pool->head if > possible. > > A bunch of the event loop APIs are thread-safe (aio_set_fd_handler(), > qemu_schedule_bh(), etc) so it's somewhat natural to make > thread_pool_submit_aio() thread-safe too. However, it would be nice to > avoid synchronization and existing callers mostly call it from the same > event loop thread that runs the BH and we can avoid locking in that > case. > > Stefan -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH] migration: re-active images when migration fails to complete

2016-12-06 Thread Dr. David Alan Gilbert
op, which would stop the cancel sneaking in early. In the case where postcopy was never enabled (!migrate_postcopy_ram()) we can move the COMPLETED transition into the lock as well; so I think then we kind of become safe. In the case where postcopy was enabled I think we can do the COMPLETED transition before waiting for the return path to close - I think but I need to think more about that. And there seem to be some dodgy cases where we call the invalidate there after a late postcopy failure; that's bad, we shouldn't reactivate the source disks after going into postcopy. So, in summary; this function is a mess - it needs a much bigger fix than this patch. Dave > Kevin > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH] migration: re-active images when migration fails to complete

2016-12-08 Thread Dr. David Alan Gilbert
* Hailiang Zhang (zhang.zhanghaili...@huawei.com) wrote: > Hi, > > On 2016/12/6 23:24, Dr. David Alan Gilbert wrote: > > * Kevin Wolf (kw...@redhat.com) wrote: > > > Am 19.11.2016 um 12:43 hat zhanghailiang geschrieben: > > > > commit fe904ea8242cbae2d

Re: [Qemu-block] [PATCH v3 1/1] migration: disallow migrate_add_blocker during migration

2016-12-09 Thread Dr. David Alan Gilbert
migration_blockers = g_slist_prepend(migration_blockers, reason); > +return 0; > +} > + > + error_setg(errp, "Cannot block a migration already in-progress"); I wonder if you need to add the 'reason' here as well; otherwise you get this message about blocking a migration but don't know what wanted to block it. So perhaps set errp from reason and then use error_prepend to add that extra message? Dave > +return -EBUSY; > } > > void migrate_del_blocker(Error *reason) > diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c > index 8ab3604..a5ba18f 100644 > --- a/stubs/migr-blocker.c > +++ b/stubs/migr-blocker.c > @@ -2,8 +2,9 @@ > #include "qemu-common.h" > #include "migration/migration.h" > > -void migrate_add_blocker(Error *reason) > +int migrate_add_blocker(Error *reason, Error **errp) > { > +return 0; > } > > void migrate_del_blocker(Error *reason) > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index f62264a..8abb54d 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -961,7 +961,11 @@ int kvm_arch_init_vcpu(CPUState *cs) > error_setg(&invtsc_mig_blocker, > "State blocked by non-migratable CPU device" > " (invtsc flag)"); > -migrate_add_blocker(invtsc_mig_blocker); > +r = migrate_add_blocker(invtsc_mig_blocker, NULL); > +if (r < 0) { > +fprintf(stderr, "migrate_add_blocker failed\n"); > +goto efail; > +} > /* for savevm */ > vmstate_x86_cpu.unmigratable = 1; > } > @@ -969,12 +973,12 @@ int kvm_arch_init_vcpu(CPUState *cs) > cpuid_data.cpuid.padding = 0; > r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data); > if (r) { > -return r; > +goto fail; > } > > r = kvm_arch_set_tsc_khz(cs); > if (r < 0) { > -return r; > +goto fail; > } > > /* vcpu's TSC frequency is either specified by user, or following > @@ -1001,6 +1005,12 @@ int kvm_arch_init_vcpu(CPUState *cs) > } > > return 0; > + > + fail: > +migrate_del_blocker(invtsc_mig_blocker); > + efail: > +error_free(invtsc_mig_blocker); > +return r; > } > > void kvm_arch_reset_vcpu(X86CPU *cpu) > -- > 2.9.3 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH v3 1/1] migration: disallow migrate_add_blocker during migration

2016-12-12 Thread Dr. David Alan Gilbert
.713e012 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -1044,6 +1044,31 @@ bool > > migration_in_postcopy_after_devices(MigrationState *s) > > return migration_in_postcopy(s) && s->postcopy_after_devices; > > } &

Re: [Qemu-block] [Qemu-devel] [PATCH] pflash_cfi01: fix per device sector length in CFI table

2017-01-16 Thread Dr. David Alan Gilbert
ure we need the nasty hack, but I'm also Ccing David for > his opinion. Hmm I don't understand enough about pflash to understand the change here; but generally if you need to keep something unchanged for older machine types, add a property and then set that property in the compa

Re: [Qemu-block] [PATCH] migration: re-active images while migration been canceled after inactive them

2017-01-23 Thread Dr. David Alan Gilbert
;to_dst_file, false); > +s->block_inactive = true; > } > } > qemu_mutex_unlock_iothread(); > @@ -1758,6 +1769,8 @@ fail_invalidate: > bdrv_invalidate_cache_all(&local_err); > if (local_err) { > error_report_err(local_err); > +} else { > +s->block_inactive = false; > } I think the fe904 commit also add the problem that this bdrv_invalidate_cache_all is done outside the big lock (Stefan and Kevin tell me bdrv_* calls generally need the lock). Dave > } > > -- > 1.8.3.1 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH] migration: re-active images while migration been canceled after inactive them

2017-01-23 Thread Dr. David Alan Gilbert
* Hailiang Zhang (zhang.zhanghaili...@huawei.com) wrote: > On 2017/1/23 21:40, Dr. David Alan Gilbert wrote: > > * zhanghailiang (zhang.zhanghaili...@huawei.com) wrote: > > > commit fe904ea8242cbae2d7e69c052c754b8f5f1ba1d6 fixed a case > > > which migration aborted QE

Re: [Qemu-block] [PATCH v2] migration: re-active images while migration been canceled after inactive them

2017-01-24 Thread Dr. David Alan Gilbert
idate_cache_all() in > qmp_migrate_cancel() > directly if we find images become inactive. > > Besides, bdrv_invalidate_cache_all() in migration_completion() doesn't have > the > protection of big lock, fix it by add the missing qemu_mutex_lock_iothread(); >

Re: [Qemu-block] [Qemu-devel] [PATCH v2] migration: re-active images while migration been canceled after inactive them

2017-01-24 Thread Dr. David Alan Gilbert
qemu_savevm_state_complete_precopy(s->to_dst_file, false); > +s->block_inactive = true; > } > } > qemu_mutex_unlock_iothread(); > @@ -1755,10 +1766,14 @@ fail_invalidate: > if (s->state == MIGRATION_STATUS_A

Re: [Qemu-block] [PATCH 03/17] migration: split common postcopy out of ram postcopy

2017-01-24 Thread Dr. David Alan Gilbert
t; These parameters are unrelated if ram postcopy is disabled. So, I should be > better to have a possibility of skipping them, then to send unneeded numbers > and write separate code to read them from the stream (rewriting > loadvm_postcopy_handle_advise to just read these two numbers and do nothing > about ram postcopy for this case). I think I'd prefer either a new command or keeping these fields (probably all 0 ?) my worry is what happens in the case if we add a 3rd postcopy subfeature; In your case we have three possibilities: a) Postcopy RAM only - 16 bytes b) Postcopy persistent-dirty-bitmap only - 0 bytes c) Both - 16 bytes Lets say we added postcopy-foo in the future and it wanted to add another 16 bytes, what would it send if it was foo+persistent-dirty-bitmap and no RAM? We'd end up with 16 bytes sent but you'd have to be very careful never to get that confused with case (a) above. (I don't feel too strongly about it though) Dave > -- > Best regards, > Vladimir > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH 03/17] migration: split common postcopy out of ram postcopy

2017-01-31 Thread Dr. David Alan Gilbert
* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > 24.01.2017 22:53, Dr. David Alan Gilbert wrote: > > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > > > 24.01.2017 12:24, Juan Quintela wrote: > > > > Vladimir Sementsov-Ogievskiy

Re: [Qemu-block] [Qemu-devel] Non-flat command line option argument syntax

2017-02-02 Thread Dr. David Alan Gilbert
that we keep clean). > So, if escaped ',' wasn't ugly and confusing enough for you... > > === Comparison === > > In my opinion, dotted keys are weird and ugly, but at least they don't > add to the quoting mess. Structured values look better, except when > they do add to the quoting mess. > > I'm having a hard time deciding which one I like less :) > > Opinions? Other ideas? Dave > > > > > [1] [PATCH v14 00/21] QAPI/QOM work for non-scalar object properties > (actually v15) > Message-Id: <1475246744-29302-1-git-send-email-berra...@redhat.com> > http://lists.gnu.org/archive/html/qemu-devel/2016-09/msg08238.html > > [2] [RFC PATCH] block: Crude initial implementation of -blockdev > Message-Id: <1485968933-9162-1-git-send-email-arm...@redhat.com> > http://lists.gnu.org/archive/html/qemu-devel/2017-02/msg00182.html > > [3] Message-ID: <87h989ncse@dusky.pond.sub.org> > http://lists.gnu.org/archive/html/qemu-devel/2016-10/msg04046.html > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] Non-flat command line option argument syntax

2017-02-03 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> = Introduction = > >> > > > > > > > >> = Structured option argument synta

Re: [Qemu-block] [Qemu-devel] Non-flat command line option argument syntax

2017-02-06 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> "Dr. David Alan Gilbert" writes: > >> > >> > * Markus Armbruster (arm...@redhat.com) wrote:

Re: [Qemu-block] [PATCH 03/17] migration: split common postcopy out of ram postcopy

2017-02-06 Thread Dr. David Alan Gilbert
* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > 01.02.2017 14:06, Vladimir Sementsov-Ogievskiy wrote: > > 24.01.2017 22:53, Dr. David Alan Gilbert wrote: > > > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > > > > 24.01.2

Re: [Qemu-block] [PATCH] block, migration: Use qemu_madvise inplace of madvise

2017-02-17 Thread Dr. David Alan Gilbert
- i.e. it's guaranteed to throw away the pages, where as posix_madvise *may* throw away the pages if the kernel feels like it. Dave > Kevin -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH] block, migration: Use qemu_madvise inplace of madvise

2017-02-17 Thread Dr. David Alan Gilbert
e_t len, int advice) > { > if (advice == QEMU_MADV_INVALID) { > errno = EINVAL; > return -1; > } > #if defined(CONFIG_MADVISE) > return madvise(addr, len, advice); > #elif defined(CONFIG_POSIX_MADVISE) > return posix_madvise(addr, len, advice); > #else > errno = EINVAL; > return -1; > #endif > } > > > > > And this is the same case. > > > > Berto > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH 15/17] iotests: add default node-name

2017-02-17 Thread Dr. David Alan Gilbert
his patch.. > > If I'm not mistaken, libvirt can be patched to explicitly set the same node > names in the QEMU command line, but that is some extra work to do there. My > point is if device names are used during migration, when available, this patch > and the libvirt change is not necessary. Always best to check with libvirt guys to see what makes sense for them; ccing in jdenemar. Dave > Fam -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH 21/24] util/cutils: Let qemu_strtosz*() optionally reject trailing crap

2017-02-20 Thread Dr. David Alan Gilbert
) passes a null @endptr. No functional > change there, because its conversion consumes the string. > > Simplify callers that use @endptr only to fail when it doesn't point > to '\0' to pass a null @endptr instead. > > Cc: Dr. David Alan Gilbert > Cc: Eduardo Habkost

Re: [Qemu-block] [PATCH 22/24] util/cutils: Return qemu_strtosz*() error and value separately

2017-02-20 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > This makes qemu_strtosz(), qemu_strtosz_mebi() and > qemu_strtosz_metric() similar to qemu_strtoi64(), except negative > values are rejected. > > Cc: Dr. David Alan Gilbert > Cc: Eduardo Habkost (maintainer:X86) > Cc: Kevin W

Re: [Qemu-block] [PATCH 23/24] util/cutils: Change qemu_strtosz*() from int64_t to uint64_t

2017-02-20 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > This will permit its use in parse_option_size(). > > Cc: Dr. David Alan Gilbert > Cc: Eduardo Habkost (maintainer:X86) > Cc: Kevin Wolf (supporter:Block layer core) > Cc: Max Reitz (supporter:Block layer core) > Cc: qemu-b

Re: [Qemu-block] [Qemu-devel] [PATCH 21/24] util/cutils: Let qemu_strtosz*() optionally reject trailing crap

2017-02-21 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> Change the qemu_strtosz() & friends to return -EINVAL when @endptr is > >> null and the conversion doesn'

Re: [Qemu-block] [PATCH 1/2] The discard flag for block stream operation

2018-10-31 Thread Dr. David Alan Gilbert
, in bytes per second > # > +# @discard: true to delete blocks duplicated in old backing files. > +# (default: false). Since 3.1. > +# > # @on-error: the action to take on an error (default report). > # 'stop' and 'enospc' can only be used if the block device > #supports io-status (see BlockInfo). Since 1.3. > @@ -2361,7 +2364,7 @@ > { 'command': 'block-stream', >'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', > '*base-node': 'str', '*backing-file': 'str', '*speed': 'int', > -'*on-error': 'BlockdevOnError', > +'*discard': 'bool', '*on-error': 'BlockdevOnError', > '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } > > ## > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning

2018-11-20 Thread Dr. David Alan Gilbert
@@ int global_state_store(void) > > void global_state_store_running(void) > > { > > const char *state = RunState_str(RUN_STATE_RUNNING); > > +assert(strlen(state) < sizeof(global_state.runstate)); > > strncpy((char *)global_state.runstate, > >

Re: [Qemu-block] [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning

2018-11-20 Thread Dr. David Alan Gilbert
* Marc-André Lureau (marcandre.lur...@redhat.com) wrote: > Hi > > On Tue, Nov 20, 2018 at 9:22 PM Dr. David Alan Gilbert > wrote: > > > > * Eric Blake (ebl...@redhat.com) wrote: > > > On 11/20/18 9:27 AM, Marc-André Lureau wrote: > > > &

Re: [Qemu-block] [PATCH 2/5] The discard flag for block stream operation

2018-11-23 Thread Dr. David Alan Gilbert
nt64_t speed = qdict_get_try_int(qdict, "speed", 0); > +bool discard = qdict_get_try_bool(qdict, "discard", false); > > qmp_block_stream(true, device, device, base != NULL, base, false, NULL, > - false, NULL, qdict_haskey(qdict, "speed"), speed

Re: [Qemu-block] [PATCH v3 4/5] migration: Use QEMU_NONSTRING for non NUL-terminated arrays

2019-01-02 Thread Dr. David Alan Gilbert
a qapi_enum_parse - so it's really treating it as a string. That code is unsafe anyway since it's assuming the received runstate would be terminated. Dave > RunState state; > bool received; > } GlobalState; > -- > 2.17.2 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH v5 3/6] The discard flag for block stream operation

2019-01-03 Thread Dr. David Alan Gilbert
on, const QDict *qdict) > const char *device = qdict_get_str(qdict, "device"); > const char *base = qdict_get_try_str(qdict, "base"); > int64_t speed = qdict_get_try_int(qdict, "speed", 0); > +bool discard = qdict_get_try_bool(qdict, "di

Re: [Qemu-block] Aborts in iotest 169

2019-01-23 Thread Dr. David Alan Gilbert
grate states. > > > It's probably an issue in the migration code and not so much in vl.c, yes... > > Yeah, I'll let the migration people jump in. Can we get a log of the qmp commands when it fails? A 'running->postmigrate' transition is a bit weird; you co

Re: [Qemu-block] Aborts in iotest 169

2019-01-23 Thread Dr. David Alan Gilbert
* Max Reitz (mre...@redhat.com) wrote: > On 23.01.19 17:35, Dr. David Alan Gilbert wrote: > > * Luiz Capitulino (lcapitul...@redhat.com) wrote: > >> On Wed, 23 Jan 2019 17:12:35 +0100 > >> Max Reitz wrote: > >> > >>> On 23.01.19 17:04, Luiz Capit

Re: [Qemu-block] [Qemu-devel] Aborts in iotest 169

2019-01-24 Thread Dr. David Alan Gilbert
ate_bitmaps) and persistent > @@ -117,12 +121,12 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase): > > # catch 'Could not reopen qcow2 layer: Bitmap already exists' > # possible error > - log = self.vm_a.get_log() > -log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) > -log = re.sub(r'^(wrote .* bytes at offset > .*\n.*KiB.*ops.*sec.*\n){3}', > - '', log) > -log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log) > -self.assertEqual(log, '') > +#log = self.vm_a.get_log() > +#log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) > +#log = re.sub(r'^(wrote .* bytes at offset > .*\n.*KiB.*ops.*sec.*\n){3}', > + #'', log) > +#log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log) > +#self.assertEqual(log, '') > > # test that bitmap is still persistent > self.vm_a.launch() > @@ -211,14 +215,16 @@ for cmb in list(itertools.product((True, False), > repeat=4)): > name += '_online' if cmb[2] else '_offline' > name += '_shared' if cmb[3] else '_nonshared' > > -inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration', > +if False: > +inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration', >*list(cmb)) > > for cmb in list(itertools.product((True, False), repeat=2)): > name = ('_' if cmb[0] else '_not_') + 'persistent_' > name += ('_' if cmb[1] else '_not_') + 'migbitmap' > > -inject_test_case(TestDirtyBitmapMigration, name, > +if name == '_persistent__migbitmap': > +inject_test_case(TestDirtyBitmapMigration, name, >'do_test_migration_resume_source', *list(cmb)) > > if __name__ == '__main__': > diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out > index 3a89159833..ae1213e6f8 100644 > --- a/tests/qemu-iotests/169.out > +++ b/tests/qemu-iotests/169.out > @@ -1,5 +1,5 @@ > - > +. > -- > -Ran 20 tests > +Ran 1 tests > > OK > > > > -- > Best regards, > Vladimir -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] Aborts in iotest 169

2019-01-24 Thread Dr. David Alan Gilbert
* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > 24.01.2019 13:10, Dr. David Alan Gilbert wrote: > > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > >> 24.01.2019 12:29, Vladimir Sementsov-Ogievskiy wrote: > >>> 23.01.2019 18:

Re: [Qemu-block] Aborts in iotest 169

2019-01-24 Thread Dr. David Alan Gilbert
s. > > However, like the problem that Dave reported a few days ago (migrating > twice leads to a crash because we try to inactivate already inactive > block devices), I think this is a symptom of a larger problem. We're > not taking the state machine serious enough. One-off checks here and > there are unlikely to ever fully solve the problem of running a command > in a runstate in which it was never supposed to run. > > I wonder whether the QAPI schema should have a field 'run-states' for > commands, and by default we would only include states where the VM has > ownership of its resources (e.g. images are activated) and which are not > temporary states that are automatically left, like finish-migrate. > > Then the default for commands is to be rejected in "unusual" runstates > where we're not expecting user intervention, and we must explicitly > allow them if they are okay, in fact. > > Instead of listing every obscure runstate, maybe we should really use > categories of runstates instead: > > 1. Running > 2. Paused, owns all resources (like disk images) > 3. Paused, doesn't own some resources (source VM after migration >completes, destination before migration completes) > 4. Paused temporarily for internal reasons (e.g. finish-migrate, >restore-vm, save-vm) > > Most commands should be okay with 1 and 2, but possibly not 3, and > almost never 4. The problem is it's tricky to get those groupings right and to figure out how they all interact. For example it's perfectly fine for most query type commands to run in (3) and (4), and you need a bunch of commands to let you recover from failures that might leave you in (3) and cont is one of those that you might use after a particular form of failed migration. Dave > Kevin -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH 1/2] qmp: forbid qmp_cont in RUN_STATE_FINISH_MIGRATE

2019-01-24 Thread Dr. David Alan Gilbert
ration. Note, it's still racy since you could call this partway through the completion before it's set to finish. Reviewed-by: Dr. David Alan Gilbert > for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { > -- > 2.18.0 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [PATCH RFC 1/2] virtio-blk: add DISCARD and WRITE ZEROES features

2019-01-24 Thread Dr. David Alan Gilbert
ite_zeroes_may_unmap = 1; > memcpy(config, &blkcfg, sizeof(struct virtio_blk_config)); > } > > @@ -787,6 +864,8 @@ static uint64_t virtio_blk_get_features(VirtIODevice > *vdev, uint64_t features, > virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY); >

Re: [Qemu-block] Aborts in iotest 169

2019-01-24 Thread Dr. David Alan Gilbert
* Kevin Wolf (kw...@redhat.com) wrote: > Am 24.01.2019 um 11:49 hat Dr. David Alan Gilbert geschrieben: > > * Kevin Wolf (kw...@redhat.com) wrote: > > > Am 24.01.2019 um 10:29 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > > 23.01.2019 18:48,

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

2016-03-19 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 03/17/2016 07:25 PM, Dr. David Alan Gilbert wrote: > > * Wen Congyang (we...@cn.fujitsu.com) wrote: > >> On 03/17/2016 06:07 PM, Alberto Garcia wrote: > >>> On Thu 17 Mar 2016 10:56:09 AM CET, Wen Congyang wrote: >

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

2016-03-19 Thread Dr. David Alan Gilbert
* Wen Congyang (we...@cn.fujitsu.com) wrote: > On 03/17/2016 05:48 PM, Dr. David Alan Gilbert wrote: > > * Wen Congyang (we...@cn.fujitsu.com) wrote: > >> On 03/17/2016 05:10 PM, Alberto Garcia wrote: > >>> On Thu 17 Mar 2016 02:22:40 AM CET, Wen Congyang > >

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

2016-03-19 Thread Dr. David Alan Gilbert
guest detects it? If the primary's local disk (children.0) fails then if we can failover at that point then the guest carries running on the secondary without ever knowing about the failure. Dave > > Thanks > Wen Congyang > > > > > Berto > > > > > > . > > > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

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

2016-03-19 Thread Dr. David Alan Gilbert
--> "children.1" (hd1.qcow2) > >>> s->children[1] <--> "children.0" (hd0.qcow2) > >> > >> Yes, it is correct. > >> > >>> > >>> Is this correct? Is this the indented behavior? Since you are reading > >>> in FIFO mode, now hd1.qcow2 will always be read first, so if > >>> children.1 was the secondary disk, it has just become the primary. > >> > >> Yes. > > > > And don't you need a way to control the order in which the disks must be > > read for COLO? > > I think in fifo mode, we should read the disk first that is added earlier. > > We don't need a way to control the order now. Can you document fully how it's used in COLO then? We should have the failure modes documented, and how you'll use it after failover etc Without that it's really difficult to tell if this naming is right. The children.0 notation is really confusing in the way that Berto describes; I hit this a couple of months ago and it really doesn't make sense. Dave > > Thanks > Wen Congyang > > > > > Berto > > > > > > . > > > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

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

2016-03-29 Thread Dr. David Alan Gilbert
* Eric Blake (ebl...@redhat.com) wrote: > On 03/29/2016 09:38 AM, Max Reitz wrote: > > On 17.03.2016 10:56, Wen Congyang wrote: > >> On 03/17/2016 05:48 PM, Dr. David Alan Gilbert wrote: > > > > [...] > > > >>> The children.0 notation is really co

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

2016-03-29 Thread Dr. David Alan Gilbert
* Max Reitz (mre...@redhat.com) wrote: > On 29.03.2016 17:50, Dr. David Alan Gilbert wrote: > > * Eric Blake (ebl...@redhat.com) wrote: > >> On 03/29/2016 09:38 AM, Max Reitz wrote: > >>> On 17.03.2016 10:56, Wen Congyang wrote: > >>>> On 03/1

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

2016-03-29 Thread Dr. David Alan Gilbert
* Max Reitz (mre...@redhat.com) wrote: > On 29.03.2016 17:54, Dr. David Alan Gilbert wrote: > > * Max Reitz (mre...@redhat.com) wrote: > >> On 29.03.2016 17:50, Dr. David Alan Gilbert wrote: > >>> * Eric Blake (ebl...@redhat.com) wrote: > >>>> On 03/

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

2016-03-29 Thread Dr. David Alan Gilbert
* Max Reitz (mre...@redhat.com) wrote: > On 29.03.2016 18:03, Dr. David Alan Gilbert wrote: > > * Max Reitz (mre...@redhat.com) wrote: > >> On 29.03.2016 17:54, Dr. David Alan Gilbert wrote: > >>> * Max Reitz (mre...@redhat.com) wrote: > >>>> On 2

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

2016-03-31 Thread Dr. David Alan Gilbert
local block device using the normal command line, and use drive_add even at startup. It solves a lot of the ordering problems with getting COLO booted. Dave > > > And the "filling up quorum's children" topic then makes me notice that > > (x-)blockdev-change shoul

Re: [Qemu-block] [Qemu-devel] [PATCH v1 1/1] qemu/migration: fix the double free problem on from_src_file

2017-06-05 Thread Dr. David Alan Gilbert
b/migration/savevm.c > @@ -2290,7 +2290,6 @@ int load_snapshot(const char *name, Error **errp) > > aio_context_acquire(aio_context); > ret = qemu_loadvm_state(f); > - qemu_fclose(f); > aio_context_release(aio_context); Thanks! Reviewed-by: Dr. David Alan Gilbert > > migration_incoming_state_destroy(); > -- > 2.11.2 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH v2 1/1] qemu/migration: fix the double free problem on from_src_file

2017-06-06 Thread Dr. David Alan Gilbert
MU_OPTIONS "$@" ) > > QEMU X.Y.Z monitor - type 'help' for more information > > -(qemu) quit > > -*** done > > +(qemu) *** done > > > > Signed-off-by: QingFeng Hao > > Reviewed-by: Dr. David Alan Gilbert > >

Re: [Qemu-block] [PATCH v2 1/1] qemu/migration: fix the double free problem on from_src_file

2017-06-07 Thread Dr. David Alan Gilbert
nds about the preferred style. Sometimes we think it's best to pass the pointer, sometimes we think it's best to call get_current. Dave > > > > Kevin > > > > -- > Regards > QingFeng Hao > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH v7 03/16] migration: split common postcopy out of ram postcopy

2017-07-11 Thread Dr. David Alan Gilbert
ese first few are pretty much a separable series). Note a few things below I'd prefer if you reroll: Reviewed-by: Dr. David Alan Gilbert > --- > migration/migration.c | 39 ++- > migration/migrat

Re: [Qemu-block] [PATCH for-2.10 06/10] hmp: Clean up and simplify hmp_migrate_set_parameter()

2017-07-18 Thread Dr. David Alan Gilbert
Bytes rather than Mebibytes. Add a comment explaining > that. > > Signed-off-by: Markus Armbruster Reviewed-by: Dr. David Alan Gilbert > --- > hmp.c | 78 > --- > 1 file changed, 32 insertions(+), 46 d

Re: [Qemu-block] [PATCH for-2.10 07/10] migration: Clean up around tls_creds, tls_hostname

2017-07-18 Thread Dr. David Alan Gilbert
tls-creds instead. > # > # @tls-hostname: hostname of the target host for the migration. This > #is required when using x509 based TLS credentials and the > @@ -1090,6 +1088,7 @@ > #certificate identity can be validated. (Since 2.7) > #An empty string means that QEMU will use the hostname > #associated with the migration URI, if any. (Since 2.9) > +#Note: 2.8 reports this by omitting tls-hostname instead. > # > # @max-bandwidth: to set maximum speed for migration. maximum speed in > # bytes per second. (Since 2.8) > -- > 2.7.5 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH for-2.10 10/10] migration: Use JSON null instead of "" to reset parameter to default

2017-07-18 Thread Dr. David Alan Gilbert
0644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -116,6 +116,13 @@ > { 'command': 'qmp_capabilities' } > > ## > +# @StrOrNull: > +## > +{ 'alternate': 'StrOrNull', > + 'data': { 's': 'str', > +'n': 'null' } } > + > +## > # @LostTickPolicy: > # > # Policy for handling lost ticks in timer devices. > @@ -1098,8 +1105,8 @@ > '*decompress-threads': 'int', > '*cpu-throttle-initial': 'int', > '*cpu-throttle-increment': 'int', > -'*tls-creds': 'str', > -'*tls-hostname': 'str', > +'*tls-creds': 'StrOrNull', > +'*tls-hostname': 'StrOrNull', > '*max-bandwidth': 'int', > '*downtime-limit': 'int', > '*x-checkpoint-delay': 'int', > -- > 2.7.5 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [RFC PATCH 03/56] monitor: Rewrite comment describing HMP .args_type

2017-08-08 Thread Dr. David Alan Gilbert
r absent (false). I found the previous description clearer. > + * TYPEs that put multiple values: > + * 'O'Option string of the form NAME=VALUE,... parsed according to > + * the QemuOptsList given by its name. > + *Example: 'device:O' uses qemu_device_opts. > + *Restriction: only lists with empty desc are supported. > + *Puts all the NAME=VALUE. > + * '/'Gdb-like print format (like "/10x"), always optional. > + *Puts keys "count", "format", "size", all int. > + * > + * Modifier character following the type string: > + * '?'Argument is optional, nothing is put when it is absent > + *(all types except 'O', '/', 'b'). > + * '.'Argument is optional, must be preceded by '.' if present > + *(only types 'i', 'l', 'M') That's obscure; I can only see one use of it in ioport_read and that's extra-special! Dave > */ > > typedef struct mon_cmd_t { > -- > 2.7.5 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [RFC PATCH 07/56] cpus: Make memsave, pmemsave sizes, addresses unsigned in QAPI/QMP

2017-08-08 Thread Dr. David Alan Gilbert
me': 'str', '*cpu-index': > 'int'} } > + 'data': {'val': 'size', 'size': 'size', 'filename': 'str', > + '*cpu-index': 'int'} } > > ## > # @pmemsave: > @@ -2489,7 +2490,7 @@ > # > ## > { 'command': 'pmemsave', > - 'data': {'val': 'int', 'size': 'int', 'filename': 'str'} } > + 'data': {'val': 'size', 'size': 'size', 'filename': 'str'} } > > ## > # @cont: > -- > 2.7.5 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [RFC PATCH 03/56] monitor: Rewrite comment describing HMP .args_type

2017-08-08 Thread Dr. David Alan Gilbert
* Paolo Bonzini (pbonz...@redhat.com) wrote: > On 08/08/2017 13:20, Dr. David Alan Gilbert wrote: > > * Markus Armbruster (arm...@redhat.com) wrote: > >> Signed-off-by: Markus Armbruster > >> --- > >> monitor.c | 75 > >> ++

Re: [Qemu-block] [RFC PATCH 09/56] balloon: Make balloon size unsigned in QAPI/QMP

2017-08-08 Thread Dr. David Alan Gilbert
+{ 'command': 'balloon', 'data': {'value': 'size'} } > > ## > # @Abort: > diff --git a/qapi/event.json b/qapi/event.json > index 6d22b02..9dfc70b 100644 > --- a/qapi/event.json > +++ b/qapi/event.json > @@ -488,7 +488,7 @@ > # > ## > { 'event': 'BALLOON_CHANGE', > - 'data': { 'actual': 'int' } } > + 'data': { 'actual': 'size' } } I was going to ask whether that was a problem for any external users, but there again libvirt looks like it reads it into an unsigned long long. Dave > ## > # @GUEST_PANICKED: > -- > 2.7.5 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [RFC PATCH 10/56] hmp: Make balloon's argument unsigned

2017-08-08 Thread Dr. David Alan Gilbert
=%" PRId64 "\n", info->actual >> 20); That looks like a partial reversion of the last patch ? Dave > qapi_free_BalloonInfo(info); > } > @@ -1178,7 +1178,7 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict) > > void hmp_balloon(Monitor *

Re: [Qemu-block] [RFC PATCH 11/56] monitor: Drop unused HMP .args_type 'M'

2017-08-08 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > The previous commit switched balloon from 'M' to 'o', rendering 'M' > unused. It was never used for anything else. Drop it. > > Signed-off-by: Markus Armbruster Reviewed-by: Dr. David Alan Gilbert >

Re: [Qemu-block] [RFC PATCH 32/56] hmp: Make block_set_io_throttle's arguments unsigned

2017-08-08 Thread Dr. David Alan Gilbert
qdict_get_int(qdict, "bps_rd"), > -.bps_wr = qdict_get_int(qdict, "bps_wr"), > +.bps = qdict_get_uint(qdict, "bps"), > +.bps_rd = qdict_get_uint(qdict, "bps_rd"), > +.bps_wr = qdict_get_uint(qdict, "bps_wr"), > .iops = qdict_get_int(qdict, "iops"), > .iops_rd = qdict_get_int(qdict, "iops_rd"), > .iops_wr = qdict_get_int(qdict, "iops_wr"), > -- > 2.7.5 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 03/56] monitor: Rewrite comment describing HMP .args_type

2017-08-08 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> Signed-off-by: Markus Armbruster > >> --- > >> monitor.c | 75 > >> ++

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 09/56] balloon: Make balloon size unsigned in QAPI/QMP

2017-08-09 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> Sizes should use QAPI type 'size' (uint64_t). balloon parameter > >> @value is 'int' (int64_t)

Re: [Qemu-block] [PATCH 2/8] io: introduce a network socket listener API

2017-08-11 Thread Dr. David Alan Gilbert
io_net_listener_channel_func, > +listener, (GDestroyNotify)object_unref); > +} > +} > + > +return data.sioc; > +} > + > +void qio_net_listener_disconnect(QIONetListener *listener) > +{ > +size_t i; > + > +if (listener->disconnected) { > +return; > +} > + > +for (i = 0; i < listener->nsioc; i++) { > +if (listener->io_tag[i]) { > +g_source_remove(listener->io_tag[i]); > +listener->io_tag[i] = 0; > +} > +qio_channel_close(QIO_CHANNEL(listener->sioc[i]), NULL); > +} > +listener->disconnected = TRUE; > +} > + > + > +gboolean qio_net_listener_is_disconnected(QIONetListener *listener) > +{ > +return listener->disconnected; > +} > + > +static void qio_net_listener_init(Object *obj) > +{ > +QIONetListener *listener = QIO_NET_LISTENER(obj); > + > +listener->disconnected = TRUE; > +} > + > +static void qio_net_listener_finalize(Object *obj) > +{ > +QIONetListener *listener = QIO_NET_LISTENER(obj); > +size_t i; > + > +qio_net_listener_disconnect(listener); > + > +for (i = 0; i < listener->nsioc; i++) { > +object_unref(OBJECT(listener->sioc[i])); > +} > +g_free(listener->io_tag); > +g_free(listener->sioc); > +g_free(listener->name); > +} > + > +static const TypeInfo qio_net_listener_info = { > +.parent = TYPE_OBJECT, > +.name = TYPE_QIO_NET_LISTENER, > +.instance_size = sizeof(QIONetListener), > +.instance_finalize = qio_net_listener_finalize, > +.instance_init = qio_net_listener_init, > +.class_size = sizeof(QIONetListenerClass), > +}; > + > + > +static void qio_net_listener_register_types(void) > +{ > +type_register_static(&qio_net_listener_info); > +} > + > + > +type_init(qio_net_listener_register_types); > -- > 2.13.3 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] [PATCH 2/8] io: introduce a network socket listener API

2017-08-11 Thread Dr. David Alan Gilbert
* Daniel P. Berrange (berra...@redhat.com) wrote: > On Fri, Aug 11, 2017 at 01:26:00PM +0100, Dr. David Alan Gilbert wrote: > > * Daniel P. Berrange (berra...@redhat.com) wrote: > > > The existing QIOChannelSocket class provides the ability to > > > listen on a single s

Re: [Qemu-block] [Qemu-devel] [PATCH] migration: flush the bdrv before stopping VM

2015-03-18 Thread Dr. David Alan Gilbert
run a script in the guest which do a dd operation, like this: > > #!/bin/sh > for i in {1..100} > do > time dd if=/dev/zero of=/time.bdf bs=4k count=20 > rm /time.bdf > done > > It's an extreme case. With what qemu options for the device, and what was your device backed by? Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

  1   2   3   4   5   6   >