Re: [Qemu-block] [Qemu-devel] [PULL 00/37] Block layer patches
On 23 October 2015 at 18:00, Kevin Wolf wrote: > The following changes since commit 1e700f4c6cddaf29ce1d205f0f8e8b9255481930: > > Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-10-23-tag' > into staging (2015-10-23 15:55:50 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to c07bc2c1658fffeee08eb46402b2f66d55b07586: > > tests: Add test case for aio_disable_external (2015-10-23 18:18:24 +0200) > > > Block layer patches > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/37] Block layer patches
On 5 November 2015 at 18:17, Kevin Wolf wrote: > The following changes since commit 8835b9df3bddf332c883c861d6a1defc12c4ebe9: > > Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-11-04-tag' > into staging (2015-11-05 10:52:35 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 22bdadd23b64af65ac2dd816848dbe2b1240a77a: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-05' > into queue-block (2015-11-05 18:01:37 +0100) > > > > Block layer patches Fails to build on OSX, I'm afraid: Undefined symbols for architecture x86_64: "_qmp_change_blockdev", referenced from: -[QemuCocoaAppController changeDeviceMedia:] in cocoa.o thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 00/40] Block layer patches
On 10 November 2015 at 14:09, Kevin Wolf wrote:
> The following changes since commit a8b4f9585a0bf5186fca793ce2c5d754cd8ec49a:
>
> Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-11-10'
> into staging (2015-11-10 09:39:24 +)
>
> are available in the git repository at:
>
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to c400bddb916268394e352f82809eb4728424a5b1:
>
> Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-10'
> into queue-block (2015-11-10 14:59:26 +0100)
>
>
>
> Block layer patches
Fails to build on OSX :-(
/Users/pm215/src/qemu-for-merges/ui/cocoa.m:1121:40: error: too few
arguments to function call, expected 7, have 5
&err);
^
./qmp-commands.h:61:1: note: 'qmp_blockdev_change_medium' declared here
void qmp_blockdev_change_medium(const char *device, const char
*filename, bool has_format, const char *format, bool
has_read_only_mode, BlockdevChangeReadOnlyMode read_only_mode, Error
**errp);
^
1 error generated.
Also some warnings:
/Users/pm215/src/qemu-for-merges/qemu-io-cmds.c:772:56: warning:
format specifies type 'size_t' (aka 'unsigned long') but the argument
has type 'unsigned long lo
ng' [-Wformat]
printf("length cannot exceed %zu, given %s\n", SIZE_MAX, argv[optind]);
~~~ ^~~~
%llu
/usr/include/stdint.h:153:20: note: expanded from macro 'SIZE_MAX'
#define SIZE_MAX UINT64_MAX
^~
/usr/include/stdint.h:87:27: note: expanded from macro 'UINT64_MAX'
#define UINT64_MAX18446744073709551615ULL
^~~
/Users/pm215/src/qemu-for-merges/qemu-io-cmds.c:1082:56: warning:
format specifies type 'size_t' (aka 'unsigned long') but the argument
has type 'unsigned long l
ong' [-Wformat]
printf("length cannot exceed %zu, given %s\n", SIZE_MAX, argv[optind]);
~~~ ^~~~
%llu
/usr/include/stdint.h:153:20: note: expanded from macro 'SIZE_MAX'
#define SIZE_MAX UINT64_MAX
^~
/usr/include/stdint.h:87:27: note: expanded from macro 'UINT64_MAX'
#define UINT64_MAX18446744073709551615ULL
^~~
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v3 00/41] Block layer patches
On 11 November 2015 at 16:08, Kevin Wolf wrote: > The following changes since commit 3c07587d49458341510360557c849e93e9afaf59: > > Merge remote-tracking branch 'remotes/dgibson/tags/ppc-next-2015' into > staging (2015-11-11 09:34:18 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 4d07c720f44beafd10907cecfd5b8a57622243c1: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-11' > into queue-block (2015-11-11 17:02:02 +0100) > > > > Block layer patches > > Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 0/1] Block patches
On 11 November 2015 at 18:00, Jeff Cody wrote: > The following changes since commit 3c07587d49458341510360557c849e93e9afaf59: > > Merge remote-tracking branch 'remotes/dgibson/tags/ppc-next-2015' into > staging (2015-11-11 09:34:18 +) > > are available in the git repository at: > > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to c833d1e8f5e95762336a823a35ade65a2d0fe587: > > gluster: allocate GlusterAIOCBs on the stack (2015-11-11 10:45:39 -0500) > > > Block patches > Applied, thanks. -- PMM
Re: [Qemu-block] [PULL v3 00/43] Block layer patches (Stefan's tree)
On 12 November 2015 at 15:37, Kevin Wolf wrote: > The following changes since commit 17e50a72a3aade0eddfebc012a5d7bdd40a03573: > > Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into > staging (2015-11-12 14:15:32 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to aece5edc96f211eec6febdafc999315a2efd: > > block: Update copyright of the accounting code (2015-11-12 16:22:47 +0100) > > > Block layer patches (rebased Stefan's pull request) Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 0/4] Block patches for 2.5.0-rc1
On 18 November 2015 at 16:08, Kevin Wolf wrote: > The following changes since commit ab9b872ab3147faf3c04e91d525815b9139dd996: > > Merge remote-tracking branch > 'remotes/mdroth/tags/qga-pull-2015-11-13-v2-tag' into staging (2015-11-18 > 12:47:29 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to ca4fa82fe66076284f702adcfe7c319ebbf909ec: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-18' > into queue-block (2015-11-18 16:27:44 +0100) > > > > Block layer patches > > > Alberto Garcia (1): > block: Call external_snapshot_clean after blockdev-snapshot > > John Snow (1): > iotests: fix race in 030 > > Kevin Wolf (1): > Merge remote-tracking branch > 'mreitz/tags/pull-block-for-kevin-2015-11-18' into queue-block > > Max Reitz (1): > blockdev: Add missing bdrv_unref() in drive-backup > > Rabin Vincent (1): > nand: fix address overflow Applied, thanks. -- PMM
Re: [Qemu-block] [PATCH] tests: fix cdrom_pio_impl in ide-test
On 20 November 2015 at 14:29, Peter Lieven wrote: > The check for the cleared BSY flag has to be performed > before each data transfer and not just before the > first one. > > Commit 5f81724d revealed this glitch as the BSY flag > was not set in ATAPI PIO transfers before. > > While at it fix the desciptions and add a comment before > the nested for loop that transfers the data. > > Signed-off-by: Peter Lieven If the IDE folks can review this I'd like to apply it direct to master this afternoon so we can tag and roll rc1 today. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH] tests: fix cdrom_pio_impl in ide-test
On 20 November 2015 at 17:12, John Snow wrote: > This looks correct. This will definitely fix the race in the test, since > it was due to a race where we were reading the data when DRQ was not set. > > Where I still remain a little confused is the precise flow control that > leads to sending an interrupt where BSY is set and DRQ is clear. > > I'd like to investigate that a little more, but for purposes of rc1 and > testing I think this is the right thing to do. > > For rc1, however: > Reviewed-by: John Snow Thanks, applied to master (I fixed the commit message typo). -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 0/6] Block patches for 2.5.0-rc2
On 25 November 2015 at 14:10, Kevin Wolf wrote: > The following changes since commit 1aae36df4b8ed884c6ef6995e70c67fad79b49df: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-ivshmem-2015-11-25' > into staging (2015-11-25 11:38:03 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 8c34d891b1594840d8394a3c9b92236c13254fd8: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-25' > into queue-block (2015-11-25 14:33:01 +0100) > > > > Block layer patches > > > Fam Zheng (1): > qemu-iotests: Add -nographic when starting QEMU in 119 and 120 > > Kevin Wolf (3): > tests/Makefile: Add more dependencies for test-timed-average > test-aio: Fix event notifier cleanup > Merge remote-tracking branch > 'mreitz/tags/pull-block-for-kevin-2015-11-25' into queue-block > > Markus Armbruster (1): > block/qapi: Plug memory leak on query-block error path > > Programmingkid (1): > raw-posix.c: Make GetBSDPath() handle caching options > > Ricard Wanderlof (1): > nand: fix flash erase when oob is in memory > > block/qapi.c | 8 +++- > block/raw-posix.c | 15 +-- > hw/block/nand.c| 2 +- > tests/Makefile | 3 +-- > tests/qemu-iotests/119 | 2 +- > tests/qemu-iotests/120 | 2 +- > tests/test-aio.c | 1 + > 7 files changed, 17 insertions(+), 16 deletions(-) Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 0/1] Block patches for 2.5
On 2 December 2015 at 14:27, Jeff Cody wrote: > The following changes since commit 680617ed43a2811318ac2df63e686f6b7bc22f55: > > Merge remote-tracking branch 'remotes/weil/tags/pull-wxx-20151130' into > staging (2015-11-30 15:35:20 +) > > are available in the git repository at: > > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to cdf45d4a6d694a394d89f0d8f72f4fefbd82be58: > > mirror: Quiesce source during "mirror_exit" (2015-11-30 16:26:14 -0500) > > > Block patches for 2.5 > > > Fam Zheng (1): > mirror: Quiesce source during "mirror_exit" > > block/mirror.c | 4 > 1 file changed, 4 insertions(+) Hi. It looks like you forgot to add your own signoff to Fam's patch. Can you respin this pullreq, please? thanks -- PMM
Re: [Qemu-block] [PULL v2 0/1] Block patches for 2.5
On 2 December 2015 at 15:47, Jeff Cody wrote: > The following changes since commit 9d7b969ea6d9663a94760c6c131481b366f4d38a: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20151201' into > staging (2015-12-02 10:16:53 +) > > are available in the git repository at: > > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to 176c36997fd4a94a7b919468d8967e0ad81fdf9c: > > mirror: Quiesce source during "mirror_exit" (2015-12-02 10:44:06 -0500) > > > Block patches for 2.5 > > > Fam Zheng (1): > mirror: Quiesce source during "mirror_exit" > > block/mirror.c | 4 > 1 file changed, 4 insertions(+) Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 0/2] Block patches for 2.5.0-rc3
On 2 December 2015 at 16:20, Kevin Wolf wrote: > The following changes since commit 9d7b969ea6d9663a94760c6c131481b366f4d38a: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20151201' into > staging (2015-12-02 10:16:53 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to ab7fe3a29ab1c6f5a1fff864e455d6d53dde62b5: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-12-02' > into queue-block (2015-12-02 16:38:03 +0100) > > > > Block layer patches > > > Kevin Wolf (2): > qcow2: Fix potential qemu-img check crash on 32 bit hosts > Merge remote-tracking branch > 'mreitz/tags/pull-block-for-kevin-2015-12-02' into queue-block > > Michael S. Tsirkin (1): > blkdebug: silence warning under qtest > > block/blkdebug.c | 9 +++-- > block/qcow2-refcount.c | 2 +- > 2 files changed, 8 insertions(+), 3 deletions(-) Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.5] virtio-blk/dataplane: parentize compat iothread
On 7 December 2015 at 16:27, Cornelia Huck wrote:
> On Mon, 7 Dec 2015 16:50:01 +0100
> Cornelia Huck wrote:
>
>> For x-data-plane=true, we create an iothread automatically for
>> compatibility. Commit d21e877 ("iothread: include id in thread name")
>> exposed that this iothread missed correct parenthood: fix this.
>>
>> Signed-off-by: Cornelia Huck
>> ---
>> hw/block/dataplane/virtio-blk.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/block/dataplane/virtio-blk.c
>> b/hw/block/dataplane/virtio-blk.c
>> index c42ddeb..c7e5668 100644
>> --- a/hw/block/dataplane/virtio-blk.c
>> +++ b/hw/block/dataplane/virtio-blk.c
>> @@ -187,6 +187,9 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev,
>> VirtIOBlkConf *conf,
>> object_initialize(&s->internal_iothread_obj,
>>sizeof(s->internal_iothread_obj),
>>TYPE_IOTHREAD);
>> +object_property_add_child(OBJECT(conf), TYPE_IOTHREAD,
>> + OBJECT(&s->internal_iothread_obj),
>> + &error_abort);
>> user_creatable_complete(OBJECT(&s->internal_iothread_obj),
>> &error_abort);
>> s->iothread = &s->internal_iothread_obj;
>> }
>
> Scratch that, does not work... (I thought I was hitting the other
> segfault in the block backend processing...)
Should I go ahead and apply Fam's "remove x-data-plane" patch then?
thanks
-- PMM
Re: [Qemu-block] [PATCH] virtio-blk: Drop x-data-plane option
On 7 December 2015 at 15:19, Paolo Bonzini wrote: > > > On 07/12/2015 14:02, Fam Zheng wrote: >> On Mon, 12/07 12:29, Cornelia Huck wrote: >>> On Mon, 7 Dec 2015 18:59:27 +0800 >>> Fam Zheng wrote: >>> The official way of enabling dataplane is through the "iothread" property that references an iothread object created by "-object iothread". Since the old "x-data-plane=on" way now even crashes, it's probably easier to just drop it: $ qemu-system-x86_64 -drive file=null-co://,id=d0,if=none \ -device virtio-blk-pci,drive=d0,x-data-plane=on ERROR:/home/fam/work/qemu/qom/object.c:1515: object_get_canonical_path_component: assertion failed: (obj->parent != NULL) Aborted >>> >>> Do we understand yet why this crashes, btw? >> >> I think it's because with x-data-plane=on, virtio-blk initialize an object >> that >> doesn't have a parent, therefore it doesn't have a valid "canonical path >> component" thing, which is different from objects created with "-object" CLI. >> I'm not very familiar with the QOM semantics here. >> >>> Signed-off-by: Fam Zheng --- hw/block/dataplane/virtio-blk.c | 15 ++- hw/block/virtio-blk.c | 1 - include/hw/virtio/virtio-blk.h | 1 - 3 files changed, 2 insertions(+), 15 deletions(-) >>> >>> No general objection to removing x-data-plane; but this probably wants >>> a mention on the changelog as x-data-plane has been described in >>> various howtos etc. over the years. >> >> Yes, that is a good point. I don't know if it's too rushing in removing it >> for >> 2.5 (this is just posted as one option) and we'll have to count on QOM >> experts >> for the fix, if it is. > > The solution would be to add object_property_add_child to > virtio_blk_data_plane_create, between object_initialize and > user_creatable_complete. But I think this patch is ok for 2.5. Paolo asked me to apply this to master, so I have done so. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.5?] qemu-iotests: Reduce racy output in 028
On 11 December 2015 at 03:27, Eric Blake wrote: > On my machine, './check -qcow2 028' was failing about 80% of the time, > due to a race in how many times the repeated attempts to run 'info > block-jobs' could occur before the job was done, showing up as a > failure of fewer '(qemu) ' prompts than in the expected output. I'm guessing from the git history for tests/qemu-iotests/028.out that this isn't a recent regression? That suggests to me that we shouldn't worry about fixing this for 2.5. PS: gmail displays your email with no line breaks in it at all, but I suspect that's gmail's fault -- perhaps it got confused by all the hardcoded ESC characters... thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.5 1/1] blockdev: Mark {insert, remove}-medium experimental
On 11 December 2015 at 15:30, Eric Blake wrote: > On 12/11/2015 08:23 AM, Max Reitz wrote: >> >> SQMP >> -blockdev-remove-medium >> +x-blockdev-remove-medium >> -- > > Formatting nit, but not worth holding this up (as it is really our last > chance to get it in 2.5 before baking in something we'd be stuck with). You mean the length of the underline, right? I can fix that up as I apply it. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.5 1/1] blockdev: Mark {insert, remove}-medium experimental
On 11 December 2015 at 15:43, Max Reitz wrote: > On 2015-12-11 at 16:40, Peter Maydell wrote: >> >> On 11 December 2015 at 15:30, Eric Blake wrote: >>> >>> On 12/11/2015 08:23 AM, Max Reitz wrote: >>>> >>>> >>>> SQMP >>>> -blockdev-remove-medium >>>> +x-blockdev-remove-medium >>>> -- >>> >>> >>> Formatting nit, but not worth holding this up (as it is really our last >>> chance to get it in 2.5 before baking in something we'd be stuck with). >> >> >> You mean the length of the underline, right? I can fix that up >> as I apply it. > > > I'd appreciate that, thanks! Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 1/4] qapi: Use Python 2.6 "except E as ..." syntax
On 17 December 2015 at 10:06, Markus Armbruster wrote: > PEP 8 calls for it, because it's forward compatibile with Python 3. Typo nit: "compatible". thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/48] Block patches
On 18 December 2015 at 15:07, Kevin Wolf wrote: > The following changes since commit 67a708406221f476c0f8fa60c192c186150c5185: > > Merge remote-tracking branch > 'remotes/berrange/tags/pull-io-channel-base-2015-12-18-1' into staging > (2015-12-18 12:42:10 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 9d4a6cf0ea471fb5aeaba9360fec863ef8a0ab44: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-12-18' > into queue-block (2015-12-18 14:40:35 +0100) > > > > Block layer patches > > Applied, thanks. -- PMM
Re: [Qemu-block] [PATCH 04/10] hw/sd: Add QOM bus which SD cards plug in to
On 20 December 2015 at 20:51, Peter Crosthwaite
wrote:
> On Sun, Dec 20, 2015 at 9:10 AM, Peter Maydell
> wrote:
>> On 19 December 2015 at 21:38, Peter Crosthwaite
>> wrote:
>>> On Fri, Dec 11, 2015 at 04:37:05PM +, Peter Maydell wrote:
>>>> +bool sdbus_get_inserted(SDBus *sdbus)
>>>> +{
>>>> +SDState *card = get_card(sdbus);
>>>> +
>>>> +if (card) {
>>>> +SDClass *sc = SD_GET_CLASS(card);
>>>> +
>>>> +return sc->get_inserted(card);
>>>> +}
>>>> +
>>>> +return false;
>>>> +}
>>>
>>> This I am not sure about. Realistically, the card has no self
>>> awareness of its ejection state so it can't be polled for "are
>>> you there". The card insertion switch is implemented as a
>>> physical switch on the slot itself and a property of the bus.
>>>
>>> The absence on presence of a device should determine this, making me
>>> think this should return !!card.
>>>
>>> Unfortunately, we have the case of the block UI being able to trigger a
>>> card ejection from underneath the bus level. But since the SD card is
>>> already
>>> using qdev_get_parent_bus() the removal from the bus can be managed at the
>>> card level.
>>
>> For user-level back compat I think we need to retain "might have
>> an sdcard object with no block backend, and that means
>> 'no-card-present'". This is both for the user facing
>> monitor commands to manipulate the sd card, and also
>
> What are the user-facing monitor commands? I tried using "change" and
> "eject", but they don't seem to work for SD, due to the tray being
> closed. Has this ever worked in a way that is user manipulatable for
> SD or is it just to handle the case of unconditional SD card creation
> (with the card never hotplugging over the system lifetime)?
I admit to just assuming that this stuff worked rather than
testing it :-)
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 10/40] blockdev: Implement change with basic operations
On 10 November 2015 at 14:09, Kevin Wolf wrote:
> From: Max Reitz
>
> Implement 'change' on block devices by calling blockdev-open-tray,
> blockdev-remove-medium, blockdev-insert-medium (a variation of that
> which does not need a node-name) and blockdev-close-tray.
>
> Signed-off-by: Max Reitz
> Signed-off-by: Kevin Wolf
I think this commit broke the monitor 'change' command for
sd card devices. In 2.4, for a Zaurus ('spitz') machine:
change sd0 /home/petmay01/test-images/RaspberryPi/pifi-4g.img
causes the guest to print
mmc0: new SDHC card at address 4567
mmcblk0: mmc0:4567 QEMU! 4.00 GiB
mmcblk0: p1 p2 p3 p4
ie it detects we have just provided a new SD card.
In 2.5 trying this gives an error in the monitor:
Tray of device 'sd0' is not open
This seems to be because with this commit we now try to do
a qmp_blockdev_open_tray() on the device. This fails for SD cards
(which don't have any kind of tray in real life), because we
don't implement the is_tray_open hook and so get the default
"tray always closed" behaviour. But in the old code you could
perfectly well change the backing medium even on a device
without a tray...
Was this breakage intentional?
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 10/40] blockdev: Implement change with basic operations
On 7 January 2016 at 19:37, Max Reitz wrote: > Compare floppy disks, for which we now have a "virtual" tray status: > Whenever a medium is inserted, the "tray" is considered closed. > Otherwise, it is open. This works pretty much like a physical tray would > work; whenever the tray is closed, you cannot exchange the medium, but > when it is open, you can. > > There is only one difference to devices which actually have a tray: For > floppy disks, you cannot have a closed tray without a medium in it. > > Anyway, we can implement the same model for SD cards. I'll see to it. It looks like sd.c is the only one which implements a change_media_cb but no is_tray_open, but it would be nice if we could implement this in the default blk_dev_is_tray_open() method rather than in the sd and floppy models (ie if I don't implement tray-open then the tray is closed if there's a medium attached, and the block backend ought to know if there's a medium attached itself already). thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 10/40] blockdev: Implement change with basic operations
On 7 January 2016 at 20:14, Max Reitz wrote: > On 07.01.2016 20:56, Peter Maydell wrote: >> It looks like sd.c is the only one which implements a change_media_cb >> but no is_tray_open, but it would be nice if we could implement this >> in the default blk_dev_is_tray_open() method rather than in the >> sd and floppy models (ie if I don't implement tray-open then the >> tray is closed if there's a medium attached, and the block backend >> ought to know if there's a medium attached itself already). > > That would be nice, but there's a difference between "there's a medium > attached" (tray can be both open and closed) and "the medium is > accessible by the guest" (tray must be closed). The BlockBackend does > not know this difference, only the guest devices does. > > It gets told of when to open/close the tray by invocation of the > change_media_cb() (the @load parameter set to false or true, > respectively), and we could track this state in the BlockBackend instead > of in the SDState. But that looks like the wrong place to me. Well, previously sd.c didn't need to have any state for this to all work right (or indeed care about implementing a fake tray status for a device that doesn't have a tray), so it seems odd that we need to invent some extra state for it to work now. > Right now, sd.c completely ignores the @load parameter of > change_media_cb(), which seems wrong; this means that "opening the tray" > keeps the medium accessible for the guest. In the past that was fine, > because eject closed the associated BDS (the medium) right afterwards, > so it actually wasn't accessible any more. The new > blockdev-open-tray no longer does that, so now we need to respect the > value of @load and no longer rely on blk_is_inserted() alone any more. I think blockdev-open-tray should probably not work for SD cards (or for floppies?), because saying "open the tray" on a device without a tray is meaningless. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 10/40] blockdev: Implement change with basic operations
On 7 January 2016 at 21:57, Max Reitz wrote: > On 07.01.2016 22:42, Peter Maydell wrote: >> Well, previously sd.c didn't need to have any state for this >> to all work right (or indeed care about implementing a fake >> tray status for a device that doesn't have a tray), so it seems >> odd that we need to invent some extra state for it to work now. > > Before, it took the state from the block layer by calling > blk_is_inserted(). Works fine as long we only have the high-level > operations change and eject. Stops to work once we introduce lower-level > operations (open-tray, remove-medium, insert-medium, close-tray). > > Why do we need the low-level operations? Mainly because they integrate > much better into the model of a more freely configurable block layer > (there aren't many options one can give to the 'change' operation; now > you can use blockdev-add and the lower-level operations afterwards). > > Why did I reimplement 'change' and 'eject' using these new operations? > Because otherwise we'd have those operations doing one kind of thing, > and the lower-level ones doing something completely different. I'd find > that bad. But these lower level operations now let you put the system conceptually into states that it really can't be in in hardware. That seems wrong to me and we should fail those commands where it makes sense (eg trying to "open a tray" when there is no tray), rather than forcing the sd.c layer code to cope with the user putting the system into impossible conditions. >>> Right now, sd.c completely ignores the @load parameter of >>> change_media_cb(), which seems wrong; this means that "opening the tray" >>> keeps the medium accessible for the guest. In the past that was fine, >>> because eject closed the associated BDS (the medium) right afterwards, >>> so it actually wasn't accessible any more. The new >>> blockdev-open-tray no longer does that, so now we need to respect the >>> value of @load and no longer rely on blk_is_inserted() alone any more. >> >> I think blockdev-open-tray should probably not work for SD cards >> (or for floppies?), because saying "open the tray" on a device >> without a tray is meaningless. > > Depends on what you think it is. For me, blockdev-open-tray generally > means "pushing the eject button". > > For actual tray devices, this means that the tray may open (or the OS > may be asked to do so). For floppy disk drives, this means the floppy > disk will be ejected. For SD slots (where there often is no dedicated > button, but one can push the SD card further in for it to get released), > this means the SD card will be ejected. > > Note that this is different from the 'eject' command which models a > drive where the user pushes the eject button and suddenly the medium > bursts into flames/is dropped on the floor/just disappears. Therefore, I > find the 'eject' command quite misnamed, but having named the new > command 'blockdev-eject-medium' wouldn't have made things any better. I don't understand what the difference is between your "floppy disk will be ejected" and "SD card will be ejected" cases, and "medium is dropped on the floor" case. Those seem like exactly the same thing to me, so we should just use "eject" and not have an extra command. I can see why we want to model tray state for devices with trays, but I don't see why this is bleeding into devices without trays. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 10/40] blockdev: Implement change with basic operations
On 7 January 2016 at 22:43, Max Reitz wrote: > I hope that the above explanation helped you understand why it bled into > tray-less devices, from a technical perspective. Yes, thanks, that was definitely a helpful explanation for why the design is the way it is. I'm still not sure how useful it is to model "floppy is in the drive slot but not pushed in" (we don't model "floppy is sat on my kitchen table", which is also a situation that a real floppy disk could be in ;-)). But I can see the attraction of trying to separate commands operating on the device from commands operating on the backend. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/21] Block layer patches
On 7 January 2016 at 22:57, Max Reitz wrote: > Kevin is on PTO this week, so I am filling in for him. > > > The following changes since commit a7e00e2536941a6e570b45b7ab4afec4505ff67e: > > petalogix-ml605: Set the MicroBlaze CPU version to 8.10.a (2016-01-07 > 14:57:26 +0100) > > are available in the git repository at: > > git://github.com/XanClic/qemu.git tags/pull-block-for-peter-2016-01-07 > > for you to fetch changes up to 94ca2c7395ef3eebb829227210c6757eba5b00d3: > > iotests: Add test cases for blockdev-mirror (2016-01-07 21:30:18 +0100) > > > Block patches from 2015-12-23 until 2016-01-07. > > > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] blk: do not select PFLASH device for internal snapshot
On 12 January 2016 at 15:13, Denis V. Lunev wrote: > The idea of this patch was trivial. First of all, I would like to keep > this image internally snapshoted. That is why the ultimate goal > was to switch from raw to qcow2 to keep changes inside the > image. > > Though in this case this drive could be selected to save VM > state, which could be big. The function being changed selects > the image for VM state saving. > > here I would like to skip IP_PFLASH from being selected to keep > it small as required by libvirt guys. This has to be a board specific decision. Some of our machine models might have no backing storage other than an IP_PFLASH drive, but it's still nice to be able to do vmsave/vmload on them. thanks -- PMM
Re: [Qemu-block] [PATCH 0/4] blockdev: Fix 'change' for slot devices
On 12 January 2016 at 15:47, Max Reitz wrote: > The series "BlockBackend and media" intended all block devices with > removable media to implement a tray model; if the devices does not have > a tray, it should emulate one. I tried this for the zaurus (spitz) board, and although "change sd0 file" now doesn't give an error, it doesn't cause the guest to notice that an SD card has been inserted either. I start the guest with no SD card plugged in, and then from the monitor issue "change sd0 file.img". The sd_cardchange callback is called (from blk_dev_change_media_cb, with load==true), but when it sd_cardchange() calls blk_is_inserted(sd->blk) this returns false and so the SD card model reports to the controller that no card is present. If I issue the command again, the callback is called twice, firstly with load=false -- this time blk_is_inserted() returns true and so the sd card says "card has been inserted" (and the guest notices this and prints its usual found-mmc-card logging). Then on the second callback with load==true blk_is_inserted() returns false again and so the card says "card not inserted". So it looks like the sense of blk_is_inserted() when the loading/unloading callbacks are made is the opposite of what the sd.c code is expecting. (You can see this behaviour with just the first 2 cc-stable patches.) thanks -- PMM
Re: [Qemu-block] [PATCH 0/4] blockdev: Fix 'change' for slot devices
On 15 January 2016 at 16:10, Peter Maydell wrote: > On 12 January 2016 at 15:47, Max Reitz wrote: >> The series "BlockBackend and media" intended all block devices with >> removable media to implement a tray model; if the devices does not have >> a tray, it should emulate one. > > I tried this for the zaurus (spitz) board, and although "change sd0 file" > now doesn't give an error, it doesn't cause the guest to notice that > an SD card has been inserted either. PS: my test image for zaurus is here: http://people.linaro.org/~peter.maydell/zaurus2.tgz You can run it with path/to/zaurus2/runme path/to/qemu-system-arm which by default will boot up with no SD card inserted, serial console output to stdout. Using the 'change sd0 somefile' ought to then make it notice an SD card and print the usual kernel messages. thanks -- PMM
[Qemu-block] [PATCH] block: Clean up includes
Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell --- block/accounting.c | 1 + block/archipelago.c | 2 +- block/backup.c | 4 +--- block/blkdebug.c| 1 + block/blkverify.c | 2 +- block/block-backend.c | 1 + block/bochs.c | 1 + block/cloop.c | 1 + block/commit.c | 1 + block/curl.c| 1 + block/dmg.c | 1 + block/gluster.c | 1 + block/io.c | 1 + block/iscsi.c | 2 +- block/linux-aio.c | 1 + block/mirror.c | 1 + block/nbd-client.c | 1 + block/nbd.c | 3 +-- block/nfs.c | 2 +- block/null.c| 1 + block/parallels.c | 1 + block/qapi.c| 1 + block/qcow.c| 1 + block/qcow2-cache.c | 3 +-- block/qcow2-cluster.c | 1 + block/qcow2-refcount.c | 1 + block/qcow2-snapshot.c | 1 + block/qcow2.c | 1 + block/qed-check.c | 1 + block/qed-cluster.c | 1 + block/qed-gencb.c | 1 + block/qed-l2-cache.c| 1 + block/qed-table.c | 1 + block/qed.c | 1 + block/quorum.c | 1 + block/raw-posix.c | 3 +-- block/raw-win32.c | 1 + block/raw_bsd.c | 1 + block/rbd.c | 2 +- block/sheepdog.c| 1 + block/snapshot.c| 1 + block/ssh.c | 4 +--- block/stream.c | 1 + block/throttle-groups.c | 1 + block/vdi.c | 1 + block/vhdx-endian.c | 1 + block/vhdx-log.c| 1 + block/vhdx.c| 1 + block/vmdk.c| 1 + block/vpc.c | 1 + block/vvfat.c | 2 +- block/win32-aio.c | 1 + block/write-threshold.c | 1 + hw/block/block.c| 1 + hw/block/cdrom.c| 1 + hw/block/dataplane/virtio-blk.c | 1 + hw/block/ecc.c | 1 + hw/block/fdc.c | 1 + hw/block/hd-geometry.c | 1 + hw/block/m25p80.c | 1 + hw/block/nvme.c | 1 + hw/block/onenand.c | 1 + hw/block/pflash_cfi01.c | 1 + hw/block/pflash_cfi02.c | 1 + hw/block/tc58128.c | 1 + hw/block/virtio-blk.c | 1 + hw/block/xen_disk.c | 12 +--- qemu-img.c | 2 +- qemu-io-cmds.c | 1 + qemu-io.c | 5 + 70 files changed, 70 insertions(+), 34 deletions(-) diff --git a/block/accounting.c b/block/accounting.c index 185025e..3f457c4 100644 --- a/block/accounting.c +++ b/block/accounting.c @@ -23,6 +23,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "block/accounting.h" #include "block/block_int.h" #include "qemu/timer.h" diff --git a/block/archipelago.c b/block/archipelago.c index 855655c..0507589 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -50,6 +50,7 @@ * */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "block/block_int.h" #include "qemu/error-report.h" @@ -59,7 +60,6 @@ #include "qapi/qmp/qjson.h" #include "qemu/atomic.h" -#include #include #include diff --git a/block/backup.c b/block/backup.c index 705bb77..00cafdb 100644 --- a/block/backup.c +++ b/block/backup.c @@ -11,9 +11,7 @@ * */ -#include -#include -#include +#include "qemu/osdep.h" #include "trace.h" #include "block/block.h" diff --git a/block/blkdebug.c b/block/blkdebug.c index 86b143d..f85c54b 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ +#include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/config-file.h" #include "block/block_int.h" diff --git a/block/blkverify.c b/block/blkverify.c index 1d75449..2a885cc 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -7,7 +7,7 @@ * See the COPYING file in the top-level directory. */ -#include +#include "qemu/osdep.h" #include "qemu/sockets.h" /* for EINPROGRESS on Windows */ #include "block/block_int.h" #include "qapi/qmp/qdict.h" diff --git a/block/block-backend.c b/block/block-backend.c index e813759..efd6146 100644 --- a/block/block-backend.c +++ b/block/block
Re: [Qemu-block] [PATCH] block: Clean up includes
On 19 January 2016 at 14:50, Kevin Wolf wrote: > Am 18.01.2016 um 19:01 hat Peter Maydell geschrieben: >> Clean up includes so that osdep.h is included first and headers >> which it implies are not included manually. >> >> This commit was created with scripts/clean-includes. >> >> Signed-off-by: Peter Maydell > >> --- a/block/archipelago.c >> +++ b/block/archipelago.c >> @@ -50,6 +50,7 @@ >> * >> */ >> >> +#include "qemu/osdep.h" >> #include "qemu-common.h" >> #include "block/block_int.h" >> #include "qemu/error-report.h" > > qemu/osdep.h is already included by qemu-common.h. Do you intend to > remove all qemu/osdep.h includes from all header files (and possibly > error out if it hasn't been included before) as soon as most/all .c > files include it explicitly? Yes, for suitable values of "as soon as". > This is what git grep says on include/: > > include/exec/cpu-defs.h:#include "qemu/osdep.h" > include/exec/helper-head.h:#include "qemu/osdep.h" > include/fpu/softfloat.h:#include "qemu/osdep.h" > include/qemu-common.h:#include "qemu/osdep.h" > include/qemu/bitmap.h:#include "qemu/osdep.h" > include/qemu/module.h:#include "qemu/osdep.h" > include/sysemu/seccomp.h:#include "qemu/osdep.h" We can't get rid of these until all C files include osdep.h as their first include. Hence these patches... thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/17] Block patches
On 20 January 2016 at 16:24, Kevin Wolf wrote: > The following changes since commit 3db34bf64ab4f8797565dd8750003156c32b301d: > > Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-peter' > into staging (2016-01-18 17:40:50 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to e9b155501da2638e4e319af9960d35da1bc8662b: > > iotests: Test that throttle values ranges (2016-01-20 13:37:57 +0100) > > > Block layer patches > > Applied, thanks. -- PMM
Re: [Qemu-block] [PATCH v2 0/4] blockdev: Fix 'change' for slot devices
On 20 January 2016 at 18:29, Max Reitz wrote:
> The series "BlockBackend and media" intended all block devices with
> removable media to implement a tray model; if the devices does not have
> a tray, it should emulate one.
> Patches 1 and 2 are CC'd to qemu-stable because they fix 'change' for SD
> card readers. Patch 3 does not fix it for floppy disk drives, it just
> changes the behavior when used on them (no TRAY_MOVED events, no
> tray_open status, and blockdev-{open,close}-tray are optional), which is
> why it is not CC'd to qemu-stable; and patch 4 is more of a change in
> behavior than a stable-worthy fix.
Thanks; this fixes the regression in sd card handling for me.
Tested-by: Peter Maydell
(including testing that just the stable-cc'd patches 1 and 2 are
sufficient to fix the regression).
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/48] Block patches
On 29 January 2016 at 17:37, Kevin Wolf wrote: > The following changes since commit 047e363b05679724d6b784c6ec6310697fe48ba0: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-softfloat-20160122' into staging (2016-01-22 > 15:19:21 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to ae873754e7b51f66f2b944f49b7baff2730ec511: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2016-01-29' > into queue-block (2016-01-29 18:11:39 +0100) > > > > Block layer patches Hi. I'm afraid this has some conflicts in hw/block/fdc.c which aren't immediately obvious how to resolve. (Looks like clash between the 'add pick_drive' patch in John's recent IDE pull and Max's tray status stuff here.) Can you fix up and resend, please? thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 00/50] Block patches
On 2 February 2016 at 17:34, Max Reitz wrote: > This pull request supersedes Kevin's pull request from Friday > (http://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg06026.html). > > > The following changes since commit d2ea854c382d4d080de1f149167e60290108f79b: > > Merge remote-tracking branch > 'remotes/berrange/tags/pull-qcrypto-next-2016-02-02-1' into staging > (2016-02-02 15:55:01 +) > > are available in the git repository at: > > git://github.com/XanClic/qemu.git tags/pull-block-for-peter-2016-02-02 > > for you to fetch changes up to 8983b670f62ab5e5e8dd2690bf8304123651bfe5: > > block: qemu-iotests - add test for snapshot, commit, snapshot bug > (2016-02-02 18:07:27 +0100) > > > Block patches > Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 06/49] virtio: move allocation to virtqueue_pop/vring_pop
On 4 February 2016 at 21:51, Michael S. Tsirkin wrote: > From: Paolo Bonzini > > The return code of virtqueue_pop/vring_pop is unused except to check for > errors or 0. We can thus easily move allocation inside the functions > and just return a pointer to the VirtQueueElement. > > The advantage is that we will be able to allocate only the space that > is needed for the actual size of the s/g list instead of the full > VIRTQUEUE_MAX_SIZE items. Currently VirtQueueElement takes about 48K > of memory, and this kind of allocation puts a lot of stress on malloc. > By cutting the size by two or three orders of magnitude, malloc can > use much more efficient algorithms. > > The patch is pretty large, but changes to each device are testable > more or less independently. Splitting it would mostly add churn. > > Signed-off-by: Paolo Bonzini > Reviewed-by: Michael S. Tsirkin > Signed-off-by: Michael S. Tsirkin > Reviewed-by: Cornelia Huck > --- > hw/9pfs/virtio-9p.h | 2 +- > include/hw/virtio/dataplane/vring.h | 2 +- > include/hw/virtio/virtio-balloon.h | 2 +- > include/hw/virtio/virtio-blk.h | 3 +- > include/hw/virtio/virtio-net.h | 2 +- > include/hw/virtio/virtio-scsi.h | 2 +- > include/hw/virtio/virtio-serial.h | 2 +- > include/hw/virtio/virtio.h | 2 +- > hw/9pfs/9p.c| 2 +- > hw/9pfs/virtio-9p-device.c | 17 > hw/block/dataplane/virtio-blk.c | 11 +++-- > hw/block/virtio-blk.c | 15 +++ > hw/char/virtio-serial-bus.c | 80 > +++-- > hw/display/virtio-gpu.c | 21 ++ > hw/input/virtio-input.c | 24 +++ > hw/net/virtio-net.c | 69 > hw/scsi/virtio-scsi-dataplane.c | 15 +++ > hw/scsi/virtio-scsi.c | 18 - > hw/virtio/dataplane/vring.c | 18 + > hw/virtio/virtio-balloon.c | 22 ++ > hw/virtio/virtio-rng.c | 10 +++-- > hw/virtio/virtio.c | 12 -- > roms/seabios| 2 +- > 23 files changed, 210 insertions(+), 143 deletions(-) > --- a/roms/seabios > +++ b/roms/seabios > @@ -1 +1 @@ > -Subproject commit 01a84bea2d28a19d2405c1ecac4bdef17683cc0c > +Subproject commit 33fbe13a3e2a01e0ba1087a8feed801a0451db21 > -- > MST Hi. This commit in this pull request includes a seabios submodule update, but the commit message says nothing about it. Is it really supposed to be here? thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/34] Block layer patches
On 22 February 2016 at 16:32, Kevin Wolf wrote: > The following changes since commit c3bce9d5f986bc22b0692a8fa9d26ce6d304375c: > > etraxfs_dma: Dont forward zero-length payload to clients (2016-02-20 > 00:17:48 +0100) > > are available in the git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to fe243e4881bc9e09767dba05f15acb016cfa7a52: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2016-02-22' > into queue-block (2016-02-22 16:57:50 +0100) > > > > Block layer patches > > Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 00/12] Block patches
On 29 February 2016 at 20:08, Jeff Cody wrote: > The following changes since commit 071608b519adf62bc29c914343a21c5407ab1ac9: > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-20160229-1' into > staging (2016-02-29 12:24:26 +) > > are available in the git repository at: > > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to cc199b16cf4cb9279aca73f5f5dce2cc337b9079: > > iotests/124: Add cluster_size mismatch test (2016-02-29 14:55:14 -0500) > > > Block patches > > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 00/34] Block layer patches
On 13 July 2016 at 13:50, Kevin Wolf wrote: > The following changes since commit ca3d87d4c84032f19478010b5604cac88b045c25: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-include-2016-07-12' > into staging (2016-07-12 16:04:36 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 543d7a42baf39c09db754ba9eca1d386e5958110: > > Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2016-07-13' > into queue-block (2016-07-13 13:45:55 +0200) > > > > Block layer patches > > Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 00/14] Block patches
On 20 July 2016 at 01:20, Jeff Cody wrote: > The following changes since commit 5d3217340adcb6c4f0e4af5d2b865331eb2ff63d: > > disas: Fix ATTRIBUTE_UNUSED define clash with ALSA headers (2016-07-19 > 16:40:39 +0100) > > are available in the git repository at: > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to 6c7189bb29de9fa2202f613f3c6caf028f96f261: > > block/gluster: add support for multiple gluster servers (2016-07-19 > 17:38:50 -0400) > > > Block pull for 2.7 > Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 0/2] Block patches for 2.7
On 26 July 2016 at 21:52, Jeff Cody wrote: > The following changes since commit f49ee630d73729ecaeecf4b38a8df11bc613914d: > > Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.7-20160726' > into staging (2016-07-26 11:53:47 +0100) > > are available in the git repository at: > > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to 0965a41e998ab820b5d660c8abfc8c819c97bc1b: > > mirror: double performance of the bulk stage if the disc is full > (2016-07-26 16:23:36 -0400) > > > Block patches for 2.7 > Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 0/3] Block layer patches for 2.7.0-rc2
On 5 August 2016 at 10:32, Kevin Wolf wrote: > The following changes since commit 42e0d60f1615ef63d16e41bb1668805560c37870: > > Merge remote-tracking branch 'remotes/riku/tags/pull-linux-user-20160804' > into staging (2016-08-04 18:36:05 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 47989f14472262a289894058f7babf1db37edda5: > > nvme: bump PCI revision (2016-08-05 10:56:08 +0200) > > > Block layer patches for 2.7.0-rc2 > > > Christoph Hellwig (2): > nvme: fix identify to be NVMe 1.1 compliant > nvme: bump PCI revision > > Kevin Wolf (1): > block: Accept any target node for transactional blockdev-backup Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 0/4] More block layer patches for 2.7.0-rc2
On 8 August 2016 at 12:52, Kevin Wolf wrote: > The following changes since commit cf5198d58088e3b416fe517b3946e5120b342761: > > Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20160805' into > staging (2016-08-08 10:39:18 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to a752e4786c19b0b368f4521a5dcbcce14882c3b1: > > iotests: fix 109 (2016-08-08 13:05:43 +0200) > > > More block layer patches for 2.7.0-rc2 Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 0/5] Block layer patches for 2.7.0-rc3
On 15 August 2016 at 14:57, Kevin Wolf wrote: > The following changes since commit 60ac136102098a70b97ab0c07bc7bf53131c: > > target-arm: Fix warn about implicit conversion (2016-08-12 11:12:24 +0100) > > are available in the git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 7d3e693646ad07459e04898663b37000858b4c20: > > iotests: Test case for wrong runtime option types (2016-08-15 15:52:29 > +0200) > > > Block layer patches for 2.7.0-rc3 > > > Max Reitz (5): > block/ssh: Use QemuOpts for runtime options > block/nbd: Use QemuOpts for runtime options > block/blkdebug: Store config filename > block/nbd: Store runtime option values > iotests: Test case for wrong runtime option types Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/42] Block layer patches
On 5 September 2016 at 19:13, Kevin Wolf wrote:
> The following changes since commit e87d397e5ef66276ccc49b829527d605ca07d0ad:
>
> Open 2.8 development tree (2016-09-05 11:38:54 +0100)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 1585512ba88324844c2722fd977b126fc2748604:
>
> coroutine: reduce stack size to 64kB (2016-09-05 19:06:49 +0200)
>
>
> Block layer patches
>
>
I'm afraid this fails 'make check' on ppc64be:
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k
--verbose -m=quick tests/test-coroutine
TEST: tests/test-coroutine... (pid=52932)
/basic/co_queue: FAIL
GTester: last random seed: R02Sc3b6f36ad285d724ab0bb9d6b3b65ff7
(pid=52934)
/basic/lifecycle:FAIL
GTester: last random seed: R02Sc30fe01c96aecf3873a5dab246c4945d
(pid=52935)
/basic/yield:FAIL
GTester: last random seed: R02Sa343d307145707796c84673870963561
(pid=52936)
/basic/nesting: FAIL
GTester: last random seed: R02S9d9c125c3934e5f3d4fd794a7a3026b6
(pid=52937)
/basic/self: FAIL
GTester: last random seed: R02Sf30817e4d44892fff95286f50d30b375
(pid=52938)
/basic/in_coroutine: FAIL
GTester: last random seed: R02S4048ae6da7f2fe5614e9cecf846f712b
(pid=52939)
/basic/order:FAIL
GTester: last random seed: R02S4261fe2d21feaee583cdd9f2a2433f5a
(pid=52940)
FAIL: tests/test-coroutine
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/42] Block layer patches
On 5 September 2016 at 19:13, Kevin Wolf wrote:
> The following changes since commit e87d397e5ef66276ccc49b829527d605ca07d0ad:
>
> Open 2.8 development tree (2016-09-05 11:38:54 +0100)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 1585512ba88324844c2722fd977b126fc2748604:
>
> coroutine: reduce stack size to 64kB (2016-09-05 19:06:49 +0200)
>
>
> Block layer patches
>
Also fails make check on 64-bit ARM:
QTEST_QEMU_BINARY=i386-softmmu/qemu-system-i386
QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MA
LLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick
tests/endianness-test te
sts/fdc-test tests/ide-test tests/ahci-test tests/hd-geo-test
tests/boot-order-test tests/bi
os-tables-test tests/pxe-test tests/rtc-test tests/ipmi-kcs-test
tests/ipmi-bt-test tests/i440fx-test tests/fw_cfg-test
tests/drive_del-test tests/wdt_ib700-test tests/tco-test
tests/e1000-test tests/e1000e-test tests/rtl8139-test tests/pcnet-test
tests/eepro100-test tests/ne2000-test tests/nvme-test tests/ac97-test
tests/es1370-test tests/virtio-net-test tests/virtio-balloon-test
tests/virtio-blk-test tests/virtio-rng-test tests/virtio-scsi-test
tests/virtio-9p-test tests/virtio-serial-test
tests/virtio-console-test tests/tpci200-test tests/ipoctal232-test
tests/display-vga-test tests/intel-hda-test tests/ivshmem-test
tests/vmxnet3-test tests/pvpanic-test tests/i82801b11-test
tests/ioh3420-test tests/usb-hcd-ohci-test tests/usb-hcd-uhci-test
tests/usb-hcd-ehci-test tests/usb-hcd-xhci-test tests/pc-cpu-test
tests/q35-test tests/test-netfilter tests/test-filter-mirror
tests/test-filter-redirector tests/postcopy-test
tests/device-introspect-test tests/qom-test
[...]
/i386/ahci/cdrom/dma/single: OK
/i386/ahci/cdrom/dma/multi: OK
/i386/ahci/cdrom/pio/single: OK
/i386/ahci/cdrom/pio/multi:
Broken pipe
FAIL
GTester: last random seed: R02Se17b2766a5bd0a0d37b43149f90e36e1
(pid=28161)
FAIL: tests/ahci-test
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/42] Block layer patches
On 5 September 2016 at 19:13, Kevin Wolf wrote: > The following changes since commit e87d397e5ef66276ccc49b829527d605ca07d0ad: > > Open 2.8 development tree (2016-09-05 11:38:54 +0100) > > are available in the git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 1585512ba88324844c2722fd977b126fc2748604: > > coroutine: reduce stack size to 64kB (2016-09-05 19:06:49 +0200) > > > Block layer patches make check on the clang sanitizer build failed too. Note that the sanitizer "runtime error" messages are not fatal, so whatever made the test fail was something else. GTESTER check-qtest-i386 /home/petmay01/linaro/qemu-for-merges/qtest.c:497:16: runtime error: null pointer passed as argument 1, which is declared to never be null /usr/include/string.h:62:62: note: nonnull attribute specified here /home/petmay01/linaro/qemu-for-merges/qtest.c:497:16: runtime error: null pointer passed as argument 1, which is declared to never be null /usr/include/string.h:62:62: note: nonnull attribute specified here Broken pipe GTester: last random seed: R02S47b95908b2d55cace56bbf395725386c Broken pipe GTester: last random seed: R02Sf7d2ad3c04d0dbe7e2835b41abe6ae94 thanks -- PMM
Re: [Qemu-block] [PULL v2 00/36] Block layer patches
On 6 September 2016 at 15:03, Kevin Wolf wrote: > The following changes since commit e87d397e5ef66276ccc49b829527d605ca07d0ad: > > Open 2.8 development tree (2016-09-05 11:38:54 +0100) > > are available in the git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to e7f98f2f92827df9944402d1613a4e32fe50215b: > > block: Allow node name for 'qemu-io' HMP command (2016-09-05 19:06:48 +0200) > > > Block layer patches Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 0/2] Block patches
On 13 September 2016 at 06:42, Jeff Cody wrote: > The following changes since commit 7263da78045dc91cc207f350911efe4259e99b3c: > > Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-signed' > into staging (2016-09-12 15:09:47 +0100) > > are available in the git repository at: > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to c76d7aab81c264e3452e778f030fb3760e5edbb9: > > qapi/block-core: add doc describing GlusterServer vs. SocketAddress > (2016-09-13 01:34:55 -0400) > > > Block patches > > > Prasanna Kumar Kalever (2): > block/gluster: add support to choose libgfapi logfile > qapi/block-core: add doc describing GlusterServer vs. SocketAddress > > block/gluster.c | 42 ++ > qapi/block-core.json | 17 - > 2 files changed, 54 insertions(+), 5 deletions(-) Applied, thanks. -- PMM
Re: [Qemu-block] [PULL 00/18] Block layer patches
On 14 September 2016 at 17:40, Max Reitz wrote: > The following changes since commit 507e4ddc3abf67391bcbc9624fd60b969c159b78: > > Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into > staging (2016-09-13 17:55:35 +0100) > > are available in the git repository at: > > git://github.com/XanClic/qemu.git tags/pull-block-2016-09-14 > > for you to fetch changes up to 262a8020cf666ae7108040683038cc46be4c48d2: > > iotest 055: refactor and speed up (2016-09-14 17:56:42 +0200) > > > Block patches for 2.8 > > Compile failure on 32 bit: CCutil/qemu-option.o /home/petmay01/qemu/util/hbitmap.c: In function 'hbitmap_serialization_size': /home/petmay01/qemu/util/hbitmap.c:439:5: error: passing argument 5 of 'serialization_chunk' from incompatible pointer type [-Werror] serialization_chunk(hb, start, count, &cur, &el_count); ^ /home/petmay01/qemu/util/hbitmap.c:410:13: note: expected 'size_t *' but argument is of type 'uint64_t *' static void serialization_chunk(const HBitmap *hb, ^ /home/petmay01/qemu/util/hbitmap.c: In function 'hbitmap_serialize_part': /home/petmay01/qemu/util/hbitmap.c:453:5: error: passing argument 5 of 'serialization_chunk' from incompatible pointer type [-Werror] serialization_chunk(hb, start, count, &cur, &el_count); ^ /home/petmay01/qemu/util/hbitmap.c:410:13: note: expected 'size_t *' but argument is of type 'uint64_t *' static void serialization_chunk(const HBitmap *hb, ^ /home/petmay01/qemu/util/hbitmap.c: In function 'hbitmap_deserialize_part': /home/petmay01/qemu/util/hbitmap.c:476:5: error: passing argument 5 of 'serialization_chunk' from incompatible pointer type [-Werror] serialization_chunk(hb, start, count, &cur, &el_count); ^ /home/petmay01/qemu/util/hbitmap.c:410:13: note: expected 'size_t *' but argument is of type 'uint64_t *' static void serialization_chunk(const HBitmap *hb, ^ /home/petmay01/qemu/util/hbitmap.c: In function 'hbitmap_deserialize_zeroes': /home/petmay01/qemu/util/hbitmap.c:505:5: error: passing argument 5 of 'serialization_chunk' from incompatible pointer type [-Werror] serialization_chunk(hb, start, count, &first, &el_count); ^ /home/petmay01/qemu/util/hbitmap.c:410:13: note: expected 'size_t *' but argument is of type 'uint64_t *' static void serialization_chunk(const HBitmap *hb, ^ cc1: all warnings being treated as errors Test failure on big-endian ppc64be: /hbitmap/serialize/granularity: OK /hbitmap/serialize/basic:** ERROR:/home/pm215/qemu/tests/test-hbitmap.c:774:hbitmap_test_serialize_range: assertion failed: (is_set) FAIL GTester: last random seed: R02Se8652df9788b7a1ec926da1717ff2d26 (pid=34146) /hbitmap/serialize/part: ** ERROR:/home/pm215/qemu/tests/test-hbitmap.c:843:test_hbitmap_serialize_part: assertion failed (should_set == test_bit(j, (unsigned long *)buf)): (1 == 0) FAIL GTester: last random seed: R02S3e07d1d6dcda6b90721e062eca26e6b9 (pid=34147) /hbitmap/serialize/zeroes: OK FAIL: tests/test-hbitmap thanks -- PMM
Re: [Qemu-block] [PULL v2 0/8] Block layer patches
On 17 September 2016 at 23:05, Max Reitz wrote:
> The following changes since commit e3571ae30cd26d19efd4554c25e32ef64d6a36b3:
>
> Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20160916' into
> staging (2016-09-16 16:54:50 +0100)
>
> are available in the git repository at:
>
> git://github.com/XanClic/qemu.git tags/pull-block-2016-09-17
>
> for you to fetch changes up to 61601533ace153922fec04460f0b9b168b244968:
>
> iotest 055: refactor and speed up (2016-09-17 23:51:18 +0200)
>
>
> Block patches for 2.8
I'm afraid this fails to build with the win32 compiler:
/home/petmay01/linaro/qemu-for-merges/block.c: In function ‘bdrv_find_format’:
/home/petmay01/linaro/qemu-for-merges/block.c:270:19: error:
comparison of unsigned expression < 0 is always false
[-Werror=type-limits]
for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
^
/home/petmay01/linaro/qemu-for-merges/block.c: In function ‘bdrv_find_protocol’:
/home/petmay01/linaro/qemu-for-merges/block.c:542:19: error:
comparison of unsigned expression < 0 is always false
[-Werror=type-limits]
for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
^
cc1: all warnings being treated as errors
thanks
-- PMM
Re: [Qemu-block] [PULL v4 0/8] Block layer patches
On 20 September 2016 at 21:43, Max Reitz wrote: > The following changes since commit a008535b9fa396226ff9cf78b8ac5f3584bda58e: > > build-sys: fix make install regression (2016-09-20 11:32:43 +0100) > > are available in the git repository at: > > git://github.com/XanClic/qemu.git tags/pull-block-2016-09-20 > > for you to fetch changes up to 819cec0114eeca80444a21f2e3526ef62d729385: > > iotest 055: refactor and speed up (2016-09-20 22:12:57 +0200) > > > Block patches for 2.8 > > Now build-tested for both Windows and Linux, both 32 and 64 bit. > Sorry for all the trouble. :-/ > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/33] Block layer patches
On 22 September 2016 at 17:29, Kevin Wolf wrote: > The following changes since commit 430da7a81d356e368ccd88dcca60f38da9aa5b9a: > > Merge remote-tracking branch 'remotes/riku/tags/pull-linux-user-20160915' > into staging (2016-09-22 15:39:54 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 5dccdc1b1e0c57746e5f90c69f697029a3686b5f: > > block: Remove BB interface from blockdev-add/del (2016-09-22 18:02:15 +0200) > > > Block layer patches > > Hi; I'm afraid this fails to build on OSX: it looks like you changed the arguments to some qmp functions but forgot to change their callsites in ui/cocoa.m. /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1088:86: error: too few arguments to function call, expected 7, have 4 qmp_eject([drive cStringUsingEncoding: NSASCIIStringEncoding], false, false, &err); ~ ^ ./qmp-commands.h:109:1: note: 'qmp_eject' declared here void qmp_eject(bool has_device, const char *device, bool has_id, const char *id, bool has_force, bool force, Error **errp); ^ /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1127:40: error: too few arguments to function call, expected 10, have 7 &err); ^ ./qmp-commands.h:61:1: note: 'qmp_blockdev_change_medium' declared here void qmp_blockdev_change_medium(bool has_device, const char *device, bool has_id, const char *id, const char *filename, bool has_format, const char *fo... ^ thanks -- PMM
Re: [Qemu-block] [PULL v2 00/33] Block layer patches
On 23 September 2016 at 13:01, Kevin Wolf wrote: > The following changes since commit e678c56f169bb576b607cda2a39c0b626ebfb221: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20160922' into staging (2016-09-22 > 18:23:14 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 9ec8873e684c2dae6fadb3a801057c613ccd2a6b: > > block: Remove BB interface from blockdev-add/del (2016-09-23 13:45:36 +0200) > > > Block layer patches > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/18] Block layer patches
On 27 September 2016 at 06:53, Kevin Wolf wrote: > The following changes since commit 7cfdc02dae0d2ff58c897496cfdbbafc0eda0f3f: > > Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging > (2016-09-26 19:47:00 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 3b856cebe5e93547852c156ca2119d075e62aed7: > > coroutine: reduce stack size to 60kB (2016-09-27 14:05:21 +0200) > > > Block layer patches > > I see 'make check' failures on x86-64 host, clang Linux: /i386/ahci/migrate/ncq/simple: OK /i386/ahci/migrate/ncq/halted: OK /i386/ahci/cdrom/dma/single: OK /i386/ahci/cdrom/dma/multi: OK /i386/ahci/cdrom/pio/single: Broken pipe FAIL GTester: last random seed: R02Sa8f729848b07c3b3e5ee67368f9d0350 (pid=10590) /i386/ahci/cdrom/pio/multi: Broken pipe FAIL GTester: last random seed: R02Se85704e04bbd382223983c878723b811 (pid=10598) FAIL: tests/ahci-test TEST: tests/hd-geo-test... (pid=10601) /i386/hd-geo/ide/none: OK thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/18] Block layer patches
On 28 September 2016 at 02:37, Kevin Wolf wrote: > [what clang config?] This is with Ubuntu Xenial's clang. configure args are '--cc=clang' '--cxx=clang++' '--enable-gtk' '--extra-cflags=-fsanitize=undefined -Werror' and the build is just 'make all -j8 && make check' in the build directory. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/18] Block layer patches
On 28 September 2016 at 02:37, Kevin Wolf wrote:
> Am 27.09.2016 um 21:42 hat Peter Maydell geschrieben:
>> On 27 September 2016 at 06:53, Kevin Wolf wrote:
>> > The following changes since commit
>> > 7cfdc02dae0d2ff58c897496cfdbbafc0eda0f3f:
>> >
>> > Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into
>> > staging (2016-09-26 19:47:00 +0100)
>> >
>> > are available in the git repository at:
>> >
>> >
>> > git://repo.or.cz/qemu/kevin.git tags/for-upstream
>> >
>> > for you to fetch changes up to 3b856cebe5e93547852c156ca2119d075e62aed7:
>> >
>> > coroutine: reduce stack size to 60kB (2016-09-27 14:05:21 +0200)
>> >
>> >
>> > Block layer patches
>> >
>> >
>>
>> I see 'make check' failures on x86-64 host, clang Linux:
>>
>> /i386/ahci/migrate/ncq/simple: OK
>> /i386/ahci/migrate/ncq/halted: OK
>> /i386/ahci/cdrom/dma/single: OK
>> /i386/ahci/cdrom/dma/multi: OK
>> /i386/ahci/cdrom/pio/single:
>> Broken pipe
>> FAIL
>> GTester: last random seed: R02Sa8f729848b07c3b3e5ee67368f9d0350
>> (pid=10590)
>> /i386/ahci/cdrom/pio/multi:
>> Broken pipe
>> FAIL
>> GTester: last random seed: R02Se85704e04bbd382223983c878723b811
>> (pid=10598)
>> FAIL: tests/ahci-test
>> TEST: tests/hd-geo-test... (pid=10601)
>> /i386/hd-geo/ide/none: OK
>
> I asked on IRC, but as you don't seem to be around at the moment, I'll
> keep things on the list instead.
I got a gdb backtrace:
Thread 1 "qemu-system-i38" received signal SIGSEGV, Segmentation fault.
0x561dea15 in address_space_translate (as=0x5a46bfc0,
addr=1106048, xlat=0x77e0d050, plen=0x77e0d058,
is_write=false) at /home/petmay01/linaro/qemu-for-merges/exec.c:423
423 {
Backtrace suggests we've run out of stack due to some infinite
recursion:
#0 0x561dea15 in address_space_translate (as=0x5a46bfc0,
addr=1106048, xlat=0x77e0d050, plen=0x77e0d058,
is_write=false) at /home/petmay01/linaro/qemu-for-merges/exec.c:423
#1 0x561edeab in address_space_map (as=,
addr=1106048, plen=, is_write=false)
at /home/petmay01/linaro/qemu-for-merges/exec.c:2909
#2 0x56840b9b in ahci_populate_sglist (as=0x5a46bfc0,
addr=1106048, dir=DMA_DIRECTION_TO_DEVICE, len=)
at /home/petmay01/linaro/qemu-for-merges/include/sysemu/dma.h:135
#3 0x56840b9b in ahci_populate_sglist (ad=,
sglist=, cmd=, limit=,
offset=1592) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:863
#4 0x56844de4 in ahci_dma_prepare_buf (dma=0x5a475b48,
limit=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1366
#5 0x5684354c in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1295
#6 0x568250cb in ide_atapi_cmd_reply_end (s=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/atapi.c:324
#7 0x56843662 in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1318
#8 0x568250cb in ide_atapi_cmd_reply_end (s=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/atapi.c:324
#9 0x56843662 in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1318
#10 0x568250cb in ide_atapi_cmd_reply_end (s=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/atapi.c:324
#11 0x56843662 in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1318
#12 0x568250cb in ide_atapi_cmd_reply_end (s=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/atapi.c:324
#13 0x56843662 in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1318
#14 0x568250cb in ide_atapi_cmd_reply_end (s=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/atapi.c:324
#15 0x56843662 in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1318
#16 0x568250cb in ide_atapi_cmd_reply_end (s=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/atapi.c:324
#17 0x56843662 in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1318
#18 0x568250cb in ide_atapi_cmd_reply_end (s=)
at /home/petmay01/linaro/qemu-for-merges/hw/ide/atapi.c:324
#19 0x56843662 in ahci_start_transfer (dma=) at
/home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:1318
#20 0x
Re: [Qemu-block] [Qemu-devel] [PULL 00/18] Block layer patches
On 29 September 2016 at 03:25, Kevin Wolf wrote: > The series contains a patch that reduces the coroutine stack size, so I > guess it's not quite infinite, but pretty deep recursion anyway. I will > drop that final patch that reduces the stack size and hope that the rest > will pass your testing (I tried some more to reproduce it, but I still > didn't manage to). Ah, I see. I suspect the clang build (since it has the sanitizer enabled) is a worst-case for stack usage. Is it possible for a guest to issue a sufficiently large request that it blows the stack, or is that capped somehow? thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL v2 00/17] Block layer patches
On 29 September 2016 at 14:12, Kevin Wolf wrote: > The following changes since commit c640f2849ee8775fe1bbd7a2772610aa77816f9f: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into > staging (2016-09-28 23:02:56 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 7d992e4d5a95d0b21c6c33bd32cef8671805e39b: > > oslib-posix: add a configure switch to debug stack usage (2016-09-29 > 14:13:39 +0200) > > > Block layer patches Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/10] Block layer patches
On 7 October 2016 at 14:42, Kevin Wolf wrote: > The following changes since commit e902754e3d0890945ddcc1b33748ed73762ddb8d: > > Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' > into staging (2016-10-06 13:34:00 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git block My pullreq-applying script thinks this isn't a signed tag. thanks -- PMM
Re: [Qemu-block] [PATCH v2] build: Work around SIZE_MAX bug in OSX headers
On 10 October 2016 at 15:38, Eric Blake wrote:
> C99 requires SIZE_MAX to be declared with the same type as the
> integral promotion of size_t, but OSX mistakenly defines it as
> an 'unsigned long long' expression even though size_t is only
> 'unsigned long'. Rather than futzing around with whether size_t
> is 32- or 64-bits wide (which would be needed if we cared about
> using SIZE_T in a #if expression), let the compiler get the right
> type for us by virtue of integer promotion.
>
> See also https://patchwork.ozlabs.org/patch/542327/ for an
> instance where the wrong type trips us up if we don't fix it
> for good in osdep.h.
>
> Some versions of glibc make a similar mistake with SSIZE_MAX; the
> goal is that the approach of this patch could be copied to work
> around that problem if it ever becomes important to us.
>
> Signed-off-by: Eric Blake
>
> ---
> v1 was here:
> https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg02520.html
>
> The topic recently came up again, and I noticed this patch sitting
> on one of my older branches, so I've taken another shot at it.
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00950.html
>
> v2: rewrite into a configure check (not sure if directly adding a
> -D to QEMU_CFLAGS is the best, so advice welcome)
Writing into config-host.mak would be preferable I think.
> I lack easy access to a Mac box, so this is untested as to whether
> it actually solves the issue...
I ran a test with one of the cast workarounds locally removed,
and this patch does allow the %zu version to build OK on OSX.
> ---
> include/qemu/osdep.h | 8
> configure| 13 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 9e9fa61..22667f6 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -141,6 +141,14 @@ extern int daemon(int, int);
> # error Unknown pointer size
> #endif
>
> +/* Mac OSX has a bug that incorrectly defines SIZE_MAX with
> + * the wrong type. Our replacement isn't usable in preprocessor
> + * expressions, but it is sufficient for our needs. */
I guess if we get bitten by this it'll be obvious (break the
OSX build) and we can fix it then.
> +#ifdef SIZE_MAX_BROKEN
> +#undef SIZE_MAX
> +#define SIZE_MAX ((sizeof(char)) * -1)
> +#endif
> +
> #ifndef MIN
> #define MIN(a, b) (((a) < (b)) ? (a) : (b))
> #endif
> diff --git a/configure b/configure
> index 5751d8e..137ebb8 100755
> --- a/configure
> +++ b/configure
> @@ -1725,6 +1725,19 @@ if test "$cocoa" = "yes"; then
> sdl=no
> fi
>
> +# Some versions of Mac OS X incorrectly define SIZE_MAX
> +cat > $TMPC << EOF
> +#include
> +#include
> +int main(int argc, char *argv[]) {
> +return printf("%zu", SIZE_MAX);
> +}
> +EOF
> +
> +if ! compile_object -Werror ; then
> +QEMU_CFLAGS="-DSIZE_MAX_BROKEN $QEMU_CFLAGS"
> +fi
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/10] Block layer patches
On 10 October 2016 at 12:37, Kevin Wolf wrote: > Am 10.10.2016 um 11:39 hat Peter Maydell geschrieben: >> On 7 October 2016 at 14:42, Kevin Wolf wrote: >> > The following changes since commit >> > e902754e3d0890945ddcc1b33748ed73762ddb8d: >> > >> > Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.8-20161006' >> > into staging (2016-10-06 13:34:00 +0100) >> > >> > are available in the git repository at: >> > >> > >> > git://repo.or.cz/qemu/kevin.git block >> >> My pullreq-applying script thinks this isn't a signed tag. > > Sorry, I forgot to actually push the tag. So your script is right, > 'block' is a branch and not a tag. Please use this one: > > git://repo.or.cz/qemu/kevin.git for-upstream Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH v5] build: Work around SIZE_MAX bug in OSX headers
On 11 October 2016 at 19:12, Eric Blake wrote:
> On 10/11/2016 01:03 PM, Programmingkid wrote:
>
>>> +/* Mac OSX has a bug that incorrectly defines SIZE_MAX with
>>> + * the wrong type. Our replacement isn't usable in preprocessor
>>> + * expressions, but it is sufficient for our needs. */
>>> +#if defined(HAVE_BROKEN_SIZE_MAX) && HAVE_BROKEN_SIZE_MAX
>>> +#undef SIZE_MAX
>>> +#define SIZE_MAX ((size_t)-1)
>>> +#endif
>>> +
>
>> I have applied your patch to the most recent git commit
>> (627eae7d729277c84f8e0ac07a8caab39c92c38d) on Mac OS 10.6.8. QEMU built
>> without any problems using gcc 4.9.
>
> Did you also tweak the code to make sure there was an instance of
> printf("%zu", SIZE_MAX) (or similar)? It's not enough that it compiles
> without complaint (although that helps), but also that the
> compiler-warning-on-printf goes away (which we currently don't have any
> in the tree, because we've been writing '"%zu", (size_t)SIZE_MAX' to
> work around the broken headers).
I have made that check, and tested that the patch causes the
resulting build failure to go away.
I'll apply this to master...
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH] hw/block/nvme: Simplify if-statements a little bit
On 12 October 2016 at 16:18, Thomas Huth wrote:
> The condition '!A || (A && B)' is equivalent to '!A || B'.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1464611
> Signed-off-by: Thomas Huth
> ---
> hw/block/nvme.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index cef3bb4..53d9d2e 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -373,7 +373,7 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd *cmd)
> if (!cqid || nvme_check_cqid(n, cqid)) {
> return NVME_INVALID_CQID | NVME_DNR;
> }
> -if (!sqid || (sqid && !nvme_check_sqid(n, sqid))) {
> +if (!sqid || !nvme_check_sqid(n, sqid)) {
> return NVME_INVALID_QID | NVME_DNR;
> }
> if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) {
> @@ -447,7 +447,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
> uint16_t qflags = le16_to_cpu(c->cq_flags);
> uint64_t prp1 = le64_to_cpu(c->prp1);
>
> -if (!cqid || (cqid && !nvme_check_cqid(n, cqid))) {
> +if (!cqid || !nvme_check_cqid(n, cqid)) {
> return NVME_INVALID_CQID | NVME_DNR;
> }
> if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) {
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH] block: nvme: correct the nvme queue id check
On 22 October 2016 at 13:09, P J P wrote:
> From: Prasad J Pandit
>
> NVME Express Controller has two queues, submission & completion
> queue. When creating a new queue object, 'nvme_create_sq' and
> 'nvme_create_cq' routines incorrectly check the queue id field.
> It could lead to an OOB access issue. Correct the queue id check
> to avoid it.
>
> Reported-by: Qinghao Tang
> Signed-off-by: Prasad J Pandit
> ---
> hw/block/nvme.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 2ded247..61bdc9d 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -373,7 +373,7 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd *cmd)
> if (!cqid || nvme_check_cqid(n, cqid)) {
> return NVME_INVALID_CQID | NVME_DNR;
> }
> -if (!sqid || (sqid && !nvme_check_sqid(n, sqid))) {
> +if (!sqid || nvme_check_sqid(n, sqid)) {
> return NVME_INVALID_QID | NVME_DNR;
> }
> if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) {
> @@ -447,7 +447,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
> uint16_t qflags = le16_to_cpu(c->cq_flags);
> uint64_t prp1 = le64_to_cpu(c->prp1);
>
> -if (!cqid || (cqid && !nvme_check_cqid(n, cqid))) {
> +if (!cqid || nvme_check_cqid(n, cqid)) {
> return NVME_INVALID_CQID | NVME_DNR;
> }
> if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) {
This looks a bit weird. Firstly, it doesn't match
the commit message, because it's still going to
call nvme_check_cqid() and nvme_check_sqid()
in the same situations, so if the commit message is
right and this is going to cause a problem then
the change doesn't seem to be sufficient.
Secondly, it's almost the same as this cleanup
patch from Thomas Huth that's already in qemu-trivial:
http://patchwork.ozlabs.org/patch/681349/
except that your version is removing the !
negations from the return value.
Can you explain in a bit more detail what the bug
is here and why this is the right fix for it?
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/23] Block layer patches
On 24 October 2016 at 18:01, Kevin Wolf wrote: > The following changes since commit a3ae21ec3fe036f536dc94cad735931777143103: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into > staging (2016-10-24 15:03:09 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 25493dc012e7c10dba51ee893b634a1dbfeed126: > > Merge remote-tracking branch 'mreitz/tags/pull-block-2016-10-24' into > queue-block (2016-10-24 18:02:26 +0200) > > > > Block layer patches Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/23] Block layer patches
On 27 October 2016 at 19:08, Kevin Wolf wrote: > The following changes since commit 5929d7e8a0e1f4bc3528b50397ae8dd0fd6b: > > Merge remote-tracking branch 'remotes/rth/tags/pull-atomic-20161026' into > staging (2016-10-27 14:06:34 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to b74fc7f78e0dd54fbae67d46552cebf81b59ae9f: > > iotests: Add test for NBD's blockdev-add interface (2016-10-27 19:05:23 > +0200) > > > Block layer patches > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/29] Block layer patches
On 31 October 2016 at 17:25, Kevin Wolf wrote: > The following changes since commit 0bb1137930f51a89fb1bfeb0c46aa68af0395167: > > Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20161031' into > staging (2016-10-31 14:48:47 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to aa2623d817e7ecb62fd917e475ccc0d42dd1a413: > > qapi: allow blockdev-add for NFS (2016-10-31 16:52:39 +0100) > > > Block layer patches > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 1/1] migration: fix compiler warning on uninitialized variable
On 31 October 2016 at 21:50, Jeff Cody wrote:
> Some older GCC versions (e.g. 4.4.7) report a warning on an
> uninitialized variable for 'request', even though all possible code
> paths that reference 'request' will be initialized. To appease
> these versions, initialize the variable to 0.
>
> Reported-by: Mark Cave-Ayland
> Signed-off-by: Jeff Cody
> ---
> migration/colo.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index e7224b8..93c85c5 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -439,7 +439,7 @@ void *colo_process_incoming_thread(void *opaque)
> }
>
> while (mis->state == MIGRATION_STATUS_COLO) {
> -int request;
> +int request = 0;
>
> colo_wait_handle_message(mis->from_src_file, &request, &local_err);
> if (local_err) {
> --
> 2.7.4
Thanks, applied to master as a buildfix.
-- PMM
Re: [Qemu-block] [PULL 00/14] Block patches for 2.8
On 31 October 2016 at 19:22, Jeff Cody wrote: > The following changes since commit 6bc56d317f7b5004ea2d89d264bddc8b4d081700: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream-mttcg' into > staging (2016-10-31 15:29:12 +) > > are available in the git repository at: > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to 4f5afc7abe382fc22bdf0ca67537d545e1044f2c: > > blockjobs: fix documentation (2016-10-31 15:22:08 -0400) > > > Block patches for 2.8 > Sorry, this doesn't apply cleanly to master, and my attempt to fix up the conflict didn't seem to work. Please can you rebase and resend? thanks -- PMM
Re: [Qemu-block] [PULL v2 00/14] Block patches for 2.8
On 1 November 2016 at 12:50, Jeff Cody wrote: > The following changes since commit bf99fd3983d7185178a0f65ce29bb94b1aecaed1: > > Merge remote-tracking branch 'remotes/rth/tags/pull-sparc-20161031-2' into > staging (2016-11-01 11:21:02 +) > > are available in the git repository at: > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to d8996368106fbf133a6e52561a34f6d0f5080446: > > blockjobs: fix documentation (2016-11-01 08:04:56 -0400) > > > Block patches for 2.8 > > > Fam Zheng (1): > block: Turn on "unmap" in active commit > > Jeff Cody (2): > qapi: add release designator to gluster logfile option > block: add gluster ifdef guard checks for SEEK_DATA/SEEK_HOLE support > > John Snow (7): > blockjobs: hide internal jobs from management API > blockjobs: Allow creating internal jobs > Replication/Blockjobs: Create replication jobs as internal > blockjob: centralize QMP event emissions > Blockjobs: Internalize user_pause logic > blockjobs: split interface into public/private, Part 1 > blockjobs: fix documentation > > Prasanna Kumar Kalever (3): > block/gluster: memory usage: use one glfs instance per volume > block/gluster: improve defense over string to int conversion > block/gluster: fix port type in the QAPI options list > > Xiubo Li (1): > rbd: make the code more readable Applied, thanks. -- PMM
Re: [Qemu-block] [PULL v2 00/14] Block patches for 2.8
On 2 November 2016 at 17:03, Stefan Hajnoczi wrote: > On Tue, Nov 01, 2016 at 08:50:57AM -0400, Jeff Cody wrote: >> The following changes since commit bf99fd3983d7185178a0f65ce29bb94b1aecaed1: >> >> Merge remote-tracking branch 'remotes/rth/tags/pull-sparc-20161031-2' into >> staging (2016-11-01 11:21:02 +) >> >> are available in the git repository at: >> >> g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > This pull request does not have a publicly-accessible URI. > > Please adjust your .gitconfig: > > [remote "public"] > url = git://github.com/codyprime/qemu.git > pushurl = g...@github.com:codyprime/qemu.git I think the apply-pullreq script can cope with that, once you've added the remote to your git repo -- there's slack in the "figure out the remote given the URL" logic to cope with github having a bunch of different ways to represent the same repo and maintainers tending to flip between them. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/17] Block patches for 2.3.0-rc0
On 16 March 2015 at 16:57, Kevin Wolf wrote: > The following changes since commit dcf848c478dd8765bd4f746fc4e80eaad44cf87d: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20150316' into staging (2015-03-16 > 13:56:10 +) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 47aced5078e13d6c1f4fbb59616654e577af8aaa: > > Merge remote-tracking branch 'mreitz/block' into queue-block (2015-03-16 > 17:11:12 +0100) > > > > Block patches for 2.3-rc0 > > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 0/3] Block patches for 2.3.0-rc1
On 19 March 2015 at 15:16, Kevin Wolf wrote: > The following changes since commit cd232acfa0d70002fed89e9293f04afda577a513: > > Update version for v2.3.0-rc0 release (2015-03-17 18:58:33 +) > > are available in the git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 5b347c541017b9ced10e8e9bce02d25bcf04c7af: > > block: Fix blockdev-backup not to use funky error class (2015-03-19 > 16:02:59 +0100) > > > Block patches for 2.3.0-rc1 > > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.3 0/1] block: New command line option --misc format-probing=off
On 23 March 2015 at 10:04, Markus Armbruster wrote: > First of all, my apologies for being so late with this. I realized > part way through the current development cycle that I couldn't do both > the error work and my half of the block probing work we discussed back > in November, so I punted the latter to the next cycle, missing the one > little feature I quite obviously could do. > > Why 2.3? > > 1. libvirt wants it, the sooner, the better. See the libvirt RFC PATCH >https://lists.nongnu.org/archive/html/qemu-devel/2015-03/msg04457.html > > 2. The patch is simple, and quite obviously does nothing unless you >run with --misc format-probing=off. I'm really dubious about adding new commandline ABI at this point in the release cycle, especially a whole new option group... -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/76] Block patches
On 28 April 2015 at 15:59, Kevin Wolf wrote: > The following changes since commit 84cbd63f87c1d246f51ec8eee5367a5588f367fd: > > Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into > staging (2015-04-28 12:22:20 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 61007b316cd71ee7333ff7a0a749a8949527575f: > > block: move I/O request processing to block/io.c (2015-04-28 15:36:17 +0200) > > > Block patches > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 0/7] Block patches
On 30 April 2015 at 20:10, Kevin Wolf wrote: > The following changes since commit 06feaacfb4cfef10cc0c93d97df7bfc8a71dbc7e: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into > staging (2015-04-30 12:04:11 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 4a4d614ff56b4cf15e83629946afe51dc116053f: > > Enable NVMe start controller for Windows guest. (2015-04-30 15:35:26 +0200) > > > Block patches > > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
On 19 May 2015 at 16:35, Kevin Wolf wrote:
> The floppy controller spec describes three different controller phases,
> which are currently not explicitly modelled in our emulation. Instead,
> each phase is represented by a combination of flags in registers.
>
> This patch makes explicit in which phase the controller currently is.
>
> Signed-off-by: Kevin Wolf
> ---
> hw/block/fdc.c | 31 +++
> 1 file changed, 31 insertions(+)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 8c41434..4d4868e 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -495,6 +495,30 @@ enum {
> FD_DIR_DSKCHG = 0x80,
> };
>
> +/*
> + * See chapter 5.0 "Controller phases" of the spec:
> + *
> + * Command phase:
> + * The host writes a command and its parameters into the FIFO. The command
> + * phase is completed when all parameters for the command have been supplied,
> + * and execution phase is entered.
> + *
> + * Execution phase:
> + * Data transfers, either DMA or non-DMA. For non-DMA transfers, the FIFO
> + * contains the payload now, otherwise it's unused. When all bytes of the
> + * required data have been transferred, the state is switched to either
> result
> + * phase (if the command produces status bytes) or directly back into the
> + * command phase for the next command.
> + *
> + * Result phase:
> + * The host reads out the FIFO, which contains one or more result bytes now.
> + */
> +typedef enum FDCtrlPhase {
> +FD_PHASE_COMMAND,
> +FD_PHASE_EXECUTION,
> +FD_PHASE_RESULT,
> +} FDCtrlPhase;
> +
> #define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
> #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
>
> @@ -504,6 +528,7 @@ struct FDCtrl {
> /* Controller state */
> QEMUTimer *result_timer;
> int dma_chann;
> +FDCtrlPhase phase;
This is new state -- don't we need to handle it on migration?
In general I'm not a huge fan of caching derived state in
device models...
-- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 4/8] fdc: Use phase in fdctrl_write_data()
On 19 May 2015 at 16:35, Kevin Wolf wrote:
> Instead of relying on a flag in the MSR to distinguish controller phases,
> use the explicit phase that we store now. Assertions of the right MSR
> flags are added.
>
> Signed-off-by: Kevin Wolf
> ---
> hw/block/fdc.c | 67
> ++
> 1 file changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 4d4868e..a13e0ce 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2001,8 +2001,10 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t
> value)
> return;
> }
> fdctrl->dsr &= ~FD_DSR_PWRDOWN;
> -/* Is it write command time ? */
> -if (fdctrl->msr & FD_MSR_NONDMA) {
> +
> +switch (fdctrl->phase) {
> +case FD_PHASE_EXECUTION:
> +assert(fdctrl->msr & FD_MSR_NONDMA);
I'm confused now. Is the fdctrl->phase really modelling hidden
(non-guest-visible) state in the floppy controller, or is it
a cache of the state that's surfaced in the msr and other bits?
This assert looks pretty weird either way. If the former,
then we should presumably be behaving however the hardware
behaves when the register bits and the phase get out of sync,
which isn't going to include assert()ing. (And assert()
provides no protection against bad guests because it might
get compiled out). On the other hand, if it's the latter
then it seems too easy for the two to get out of sync due
to code bugs. The usual approach there is either to
(1) have a function that updates everything at once or
(2) don't actually keep the register bits in fdctrl->msr
but just calculate them from fdctrl->phase on register
reads.
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
On 19 May 2015 at 21:52, John Snow wrote: > Hmm, I think this is not purely derived state because the flags are not > necessarily sufficient for regenerating that state. Yeah, if there's genuinely an underlying state machine that's not completely visible in registers you need to actually model it. You should probably then model the register bits by calculating them from the state rather than by changing them as you go along in parallel with moving the state machine around. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
On 20 May 2015 at 08:54, Kevin Wolf wrote: > Am 19.05.2015 um 22:57 hat Peter Maydell geschrieben: >> Yeah, if there's genuinely an underlying state machine that's >> not completely visible in registers you need to actually model it. >> You should probably then model the register bits by calculating >> them from the state rather than by changing them as you go along >> in parallel with moving the state machine around. > > I think the combination of registers is actually enough to reconstruct > what state we're in, so it is derived (otherwise I would have fixed a > bug that I'm not aware of). Adding logic to derive it in a post-load > handler should be good enough. That handles migration, which is good. But I still think that storing the same information in two places in the device state (phase field and the register fields) is error-prone. If we want to switch to having a phase field we should calculate the relevant register bits on demand based on the phase, rather than keeping both copies of the state in sync manually. thanks -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
On 20 May 2015 at 09:43, Kevin Wolf wrote: > Am 20.05.2015 um 10:06 hat Peter Maydell geschrieben: >> That handles migration, which is good. But I still think that >> storing the same information in two places in the device >> state (phase field and the register fields) is error-prone. > > That's actually my point. The registers are accessed everywhere in the > code, whereas phase transitions are in very few well-defined places > (there are exactly four of them, iirc). If they get out of sync, chances > are that the bug is in the register value, not in the phase. When we > know what phase we're in, we can assert the bits and actually catch such > bugs. > >> If we want to switch to having a phase field we should calculate >> the relevant register bits on demand based on the phase, rather >> than keeping both copies of the state in sync manually. > > That doesn't work, unfortunately. Some register bits imply a specific > phase (assuming correct code), but you can't derive the exact bits just > from the phase. Having now dug out a copy of the 82078 spec, I agree that the state isn't derivable purely from the register values in the general case. The controller clearly has a state machine internally but it doesn't surface that in the register state except indirectly. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
On 20 May 2015 at 12:55, John Snow wrote: > So even if /currently/ we can reconstitute it from the register values, > we may eventually be unable to. > > post_load will work for now, but I fear the case (in ten years) when > someone else cleans up FDC code but fails to realize that the phase is > not explicitly migrated. I assumed we would only do the post-load thing if the source for migration doesn't migrate phase (however we phrase that in a vmstate struct). -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
On 21 May 2015 at 10:42, Kevin Wolf wrote: > Am 20.05.2015 um 14:07 hat Peter Maydell geschrieben: >> On 20 May 2015 at 12:55, John Snow wrote: >> > So even if /currently/ we can reconstitute it from the register values, >> > we may eventually be unable to. >> > >> > post_load will work for now, but I fear the case (in ten years) when >> > someone else cleans up FDC code but fails to realize that the phase is >> > not explicitly migrated. >> >> I assumed we would only do the post-load thing if the >> source for migration doesn't migrate phase (however we >> phrase that in a vmstate struct). > > I think if we extend the VMState, then we have two options. I'm not > exactly sure how they work in details, but I'll try an educated guess - > Juan, please correct me if I'm wrong: > > 1. We increase version_id and add the new field at the end. This breaks >backwards migration; on forward migration the new field would be >initialised with 0 and a post_load handler could check the old >version_id to calculate the phase from register bits. > > 2. We add a subsection for the phase, and declare one phase to be the >default (most likely the command phase) for which a subsection is not >sent. In this case, the destination can't distinguish between a >missing subsection because the source was running an old qemu or >because it is the default phase. Unclear whether post_load should >recalculate the phase or not. 2b add a subsection, send the subsection always in new qemu, if receiving from an old qemu then calculate the phase from the register fields in post-load That's like 1 but just using a subsection rather than a new field. (I have a vague recollection that distros doing backports prefer subsections over increment-version-id-and-add-field). > If the above is correct, I'm afraid that the third option - which > doesn't address John's (valid) concerns - would be the most reasonable: > > 3. Don't add any VMState fields now and just do the post_load handler. >If we ever extend fdc in a way that makes it impossible to >reconstruct the phase from other migrated state, we add a subsection >that is only sent in cases where it differs from the reconstructed >value. I'm definitely against this one. State should always be sent in migration, and not doing that once we've added it to the device struct is a bug. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase
On 21 May 2015 at 12:09, Markus Armbruster wrote: > Kevin Wolf writes: > >> Am 21.05.2015 um 12:11 hat Peter Maydell geschrieben: >>> 2b add a subsection, send the subsection always in new qemu, >>> if receiving from an old qemu then calculate the phase from >>> the register fields in post-load > > Doesn't this break new -> old migration? Do we care about that? Personally I would have thought it was too hard to defend and a very narrow use case compared to old->new. -- PMM
Re: [Qemu-block] [Qemu-devel] [PULL 00/22] Block layer core and image format patches
On 22 May 2015 at 16:26, Kevin Wolf wrote: > The following changes since commit 8b6db32a4ec47d1171ccfa21d557096b99f4eef0: > > Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' > into staging (2015-05-22 13:25:40 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 4120201d2fcfc24404fe6eb6b761b66bc35bca16: > > MAINTAINERS: Split "Block QAPI, monitor, command line" off core (2015-05-22 > 17:08:09 +0200) > > > Block layer core and image format patches > Applied, thanks. -- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl->phase
On 29 May 2015 at 11:34, Dr. David Alan Gilbert wrote: > It's the destination I'm worried about here, not the source; lets say > you have two devices, a & b. 'a' gets serialised, but then 'b' finds > it has to wait, so we return to running the source and sending pages > across. However the destination has already loaded the 'a' device state; > so that when we serialise again we send a new 'a' device state'; I'm > worrying here that the destination 'a' state loader would get upset > trying to load the same state twice without somehow resetting it. You would need to reset the system before resending device state, I think. Device implementations rely on knowing that migration happens into a freshly reset device, and we don't have any way of resetting a single device, only the whole system at once. -- PMM
[Qemu-block] [PATCH 4/4] hw/arm/virt: Make block devices default to virtio
Now we have virtio-pci, we can make the virt board's default block
device type be IF_VIRTIO. This allows users to use simplified
command lines that don't have to explicitly create virtio-pci-blk
devices; the -hda &c very short options now also work.
Signed-off-by: Peter Maydell
---
hw/arm/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0a75cc8..14afe86 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -961,6 +961,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
mc->init = machvirt_init;
mc->max_cpus = 8;
mc->has_dynamic_sysbus = true;
+mc->block_default_type = IF_VIRTIO;
}
static const TypeInfo machvirt_info = {
--
1.9.1
[Qemu-block] [PATCH 3/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
If a user requests an IF_VIRTIO drive on the command line, don't
create the implicit PCI virtio device immediately, but wait until
the rest of the command line has been processed and only create the
device if the drive would otherwise be orphaned. This means that
if the user said drive,id=something,... -device drive=something,..,.
we'll allow the drive to be connected to the user's specified
device rather than stealing it to connect to the implicit virtio
device.
This deferral is *not* done for S390 devices, purely to ensure that
we retain backwards compatibility with command lines. We can
change the behaviour for PCI because right now no machine
specifies a block_default_type of IF_VIRTIO except for the
S390 machines.
Signed-off-by: Peter Maydell
---
blockdev.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 177b285..c480f64 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -46,6 +46,7 @@
#include "qmp-commands.h"
#include "trace.h"
#include "sysemu/arch_init.h"
+#include "monitor/qdev.h"
static const char *const if_name[IF_COUNT] = {
[IF_NONE] = "none",
@@ -236,6 +237,17 @@ static void create_implicit_virtio_device(const char
*driveid,
if (devaddr) {
qemu_opt_set(devopts, "addr", devaddr, &error_abort);
}
+
+if (done_orphan_check) {
+/* If we're called after vl.c has processed the -device options
+ * then we need to create the device ourselves now.
+ */
+DeviceState *dev = qdev_device_add(devopts);
+if (!dev) {
+exit(1);
+}
+object_unref(OBJECT(dev));
+}
}
bool drive_check_orphaned(void)
@@ -252,6 +264,14 @@ bool drive_check_orphaned(void)
/* Unless this is a default drive, this may be an oversight. */
if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
dinfo->type != IF_NONE) {
+if (dinfo->type == IF_VIRTIO) {
+/* An orphaned virtio drive might be waiting for us to
+ * create the implicit device for it.
+ */
+create_implicit_virtio_device(blk_name(blk), dinfo->devaddr);
+continue;
+}
+
fprintf(stderr, "Warning: Orphaned drive without device: "
"id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
blk_name(blk), blk_bs(blk)->filename, if_name[dinfo->type],
@@ -956,8 +976,13 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
goto fail;
}
-if (type == IF_VIRTIO && !done_orphan_check) {
-/* Drives created via the monitor (hotplugged) do not get the
+if (type == IF_VIRTIO && !done_orphan_check
+&& arch_type == QEMU_ARCH_S390X) {
+/* Virtio drives created on the command line get an implicit device
+ * created for them. For non-s390x command line drives, the creation
+ * of the implicit device is deferred to drive_check_orphaned. (S390x
+ * is special-cased purely for backwards compatibility.)
+ * Drives created via the monitor (hotplugged) do not get the
* magic implicit device created for them.
*/
create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
--
1.9.1
[Qemu-block] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
blockdev.c will create implicit virtio-blk-* devices for IF_VIRTIO drives. I want to turn this on for the ARM virt board (now it has PCI), so that users can use shorter and more comprehensible command lines. Unfortunately, the code as it stands will always create an implicit device, which means that setting the virt block_default_type to IF_VIRTIO would break existing command lines like: -drive file=arm-wheezy.qcow2,id=foo -device virtio-blk-pci,drive=foo because the creation of the drive causes us to create an implied virtio-blk-pci which steals the drive, and then the explicit device creation fails because 'foo' is already in use. This patchset fixes this problem by deferring the creation of the implicit devices to drive_check_orphaned(), which means we can do it only for the drives which haven't been explicitly connected to a device by the user. The slight downside of deferral is that it is effectively rearranging the order in which the devices are created, which means they will end up in different PCI slots, etc. We can get away with this for PCI, because at the moment the only machines which set their block_default_type to IF_VIRTIO are the S390X ones. I have special-cased S390X to retain the old behaviour, which is a bit ugly but functional. If S390X doesn't care about cross-version migration we can probably drop that and just have everything defer. S390X people? The code in master didn't seem to take much account of the possibility of hotplug -- if the user created a drive via the monitor we would apparently try to create the implicit drive, but in fact not do so because vl.c had already done device creation long before. I've included a patch that makes it more explicit that hotplug does not get you the magic implicit devices. The last patch is the oneliner to enable the default for virt once the underlying stuff lets us do this without breaking existing user command lines. Peter Maydell (4): blockdev: Factor out create_implicit_virtio_device blockdev: Don't call create_implicit_virtio_device() when it has no effect blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives hw/arm/virt: Make block devices default to virtio blockdev.c| 72 +++ hw/arm/virt.c | 1 + 2 files changed, 59 insertions(+), 14 deletions(-) -- 1.9.1
[Qemu-block] [PATCH 1/4] blockdev: Factor out create_implicit_virtio_device
Factor out the code which creates the implicit virtio device
for IF_VIRTIO drives, so we can call it from drive_check_orphaned().
Signed-off-by: Peter Maydell
---
blockdev.c | 35 ++-
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index de94a8b..9cf6123 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -212,6 +212,27 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int
unit)
return NULL;
}
+/* Create the virtio device whose existence is implied by the
+ * creation of a block device with if=virtio.
+ */
+static void create_implicit_virtio_device(const char *driveid,
+ const char *devaddr)
+{
+QemuOpts *devopts;
+
+devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ &error_abort);
+if (arch_type == QEMU_ARCH_S390X) {
+qemu_opt_set(devopts, "driver", "virtio-blk-s390", &error_abort);
+} else {
+qemu_opt_set(devopts, "driver", "virtio-blk-pci", &error_abort);
+}
+qemu_opt_set(devopts, "drive", driveid, &error_abort);
+if (devaddr) {
+qemu_opt_set(devopts, "addr", devaddr, &error_abort);
+}
+}
+
bool drive_check_orphaned(void)
{
BlockBackend *blk;
@@ -929,19 +950,7 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
}
if (type == IF_VIRTIO) {
-QemuOpts *devopts;
-devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
- &error_abort);
-if (arch_type == QEMU_ARCH_S390X) {
-qemu_opt_set(devopts, "driver", "virtio-blk-s390", &error_abort);
-} else {
-qemu_opt_set(devopts, "driver", "virtio-blk-pci", &error_abort);
-}
-qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
- &error_abort);
-if (devaddr) {
-qemu_opt_set(devopts, "addr", devaddr, &error_abort);
-}
+create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
}
filename = qemu_opt_get(legacy_opts, "file");
--
1.9.1
[Qemu-block] [PATCH 2/4] blockdev: Don't call create_implicit_virtio_device() when it has no effect
create_implicit_virtio_device() just adds a device to the "device" QEMU
options list. This means that it only has an effect if it is called
before vl.c processes that list to create all the devices. If it is
called after that (ie for hotplug drives created from the monitor) then
it has no effect. To avoid confusion, don't call the code at all if
it isn't going to do anything.
Signed-off-by: Peter Maydell
---
blockdev.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 9cf6123..177b285 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -78,6 +78,11 @@ static int if_max_devs[IF_COUNT] = {
[IF_SCSI] = 7,
};
+/* True once we've created all the command line drives and run
+ * drive_check_orphaned().
+ */
+static bool done_orphan_check;
+
/**
* Boards may call this to offer board-by-board overrides
* of the default, global values.
@@ -239,6 +244,8 @@ bool drive_check_orphaned(void)
DriveInfo *dinfo;
bool rs = false;
+done_orphan_check = true;
+
for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
dinfo = blk_legacy_dinfo(blk);
/* If dinfo->bdrv->dev is NULL, it has no device attached. */
@@ -949,7 +956,10 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
goto fail;
}
-if (type == IF_VIRTIO) {
+if (type == IF_VIRTIO && !done_orphan_check) {
+/* Drives created via the monitor (hotplugged) do not get the
+ * magic implicit device created for them.
+ */
create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
}
--
1.9.1
Re: [Qemu-block] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
On 8 June 2015 at 09:18, Christian Borntraeger wrote:
> Am 08.06.2015 um 10:02 schrieb Christian Borntraeger:
>> So I would prefer to not have this workaround and doing
>>
>> index c480f64..7627d57 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -976,17 +976,6 @@ DriveInfo *drive_new(QemuOpts *all_opts,
>> BlockInterfaceType block_default_type)
>> goto fail;
>> }
>>
>> -if (type == IF_VIRTIO && !done_orphan_check
>> -&& arch_type == QEMU_ARCH_S390X) {
>> -/* Virtio drives created on the command line get an implicit device
>> - * created for them. For non-s390x command line drives, the creation
>> - * of the implicit device is deferred to drive_check_orphaned.
>> (S390x
>> - * is special-cased purely for backwards compatibility.)
>> - * Drives created via the monitor (hotplugged) do not get the
>> - * magic implicit device created for them.
>> - */
>> -create_implicit_virtio_device(qdict_get_str(bs_opts, "id"),
>> devaddr);
>> -}
>>
>> filename = qemu_opt_get(legacy_opts, "file");
>>
>> actually enables the first command line to work as well.
>> So lets get rid of the s390 special handling
Cool, that certainly makes the code nicer not to have to keep the
special case. I'll respin with that change folded in.
>> (but I really appreciate that you cared about s390)
Non-x86 architectures have to stick together, you know :-)
> As a side note, I cannot test this with libvirt right now, as current qemu
> broke
> libvirts capability query
> see
> https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg01806.html
> https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg01488.html
There's a fix for that that I've just now committed to master.
thanks
-- PMM
Re: [Qemu-block] [Qemu-devel] [PATCH 0/4] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
On 8 June 2015 at 15:18, Markus Armbruster wrote: > Peter Maydell writes: > >> blockdev.c will create implicit virtio-blk-* devices for IF_VIRTIO >> drives. I want to turn this on for the ARM virt board (now it has PCI), >> so that users can use shorter and more comprehensible command lines. > > I had to read further until understood your goal: you want to make > -drive default to if=virtio for this machine type. It currently > defaults to if=ide, but the board doesn't pick those up. > >> Unfortunately, the code as it stands will always create an implicit >> device, which means that setting the virt block_default_type to IF_VIRTIO >> would break existing command lines like: >> >> -drive file=arm-wheezy.qcow2,id=foo >> -device virtio-blk-pci,drive=foo > > This works basically by accident. The documented way to do it is -drive > if=none. Omitting if= like you do defaults to the board's > block_default_type, in this case if=ide. Yes, but since we didn't reject these command lines, they worked anyway. (We should ideally have rejected them as "hey you asked for an IDE drive but it wasn't connected to anything".) So we should avoid breaking them now. > Since the drive remains unconnected, -device using it succeeds, even > though if=ide. -device should really require if=none. But since it has > never bothered to, this misuse may have hardened into de facto ABI. > > Same for any block_default_type other than IF_NONE. Indeed. >> This patchset fixes this problem by deferring the creation of the >> implicit devices to drive_check_orphaned(), which means we can do >> it only for the drives which haven't been explicitly connected to >> a device by the user. > > This changes QEMU from rejecting a certain pattern of incorrect usage to > guessing what the user meant. Example: > > $ qemu-system-x86_64 -nodefaults -display none -drive > if=virtio,file=tmp.qcow2,id=foo -device ide-hd,drive=foo > > The user asks for the drive to be both a virtio and an IDE device, which > is of course nonsense. > > Before your patch, we reject the nonsense: > > qemu-system-x86_64: -drive if=virtio,file=tmp.qcow2,id=foo: Property > 'virtio-blk-device.drive' can't take value 'foo', it's in use > > Afterwards, we accept it, guessing the user really meant -device ide=hd, > and not if=virtio. > > But if you try the same with if=scsi, we still reject it. > > Is this really a good idea? We're only rejecting it by accident, I think, because the PC machine happens to support SCSI. If you remove the pc_pci_device_init() code in pc.c which creates SCSI adaptors, then we will happily accept the command line and plug the if=scsi drive into the IDE adaptor. Similarly, the pc machine will currently accept the combination -drive if=sd,...,id=foo -device ide=hd,drive=foo If we care about catching mismatched drive-types and devices, we should actually check that, rather than rejecting them if the machine supports devices for that drive type and ignoring the mismatch if it does not. >> The slight downside of deferral is that it is effectively rearranging >> the order in which the devices are created, which means they will >> end up in different PCI slots, etc. We can get away with this for PCI, >> because at the moment the only machines which set their block_default_type >> to IF_VIRTIO are the S390X ones. I have special-cased S390X to retain >> the old behaviour, which is a bit ugly but functional. If S390X doesn't >> care about cross-version migration we can probably drop that and >> just have everything defer. S390X people? >> >> The code in master didn't seem to take much account of the possibility >> of hotplug -- if the user created a drive via the monitor > > Full monitor command, please? I haven't tested creating anything via the monitor. I merely noted that the other code path calling drive_new() is via hmp_drive_add(). (I notice now that that will actually fail on anything other than an IF_NONE drive, so it's just as well we weren't creating devices for IF_VIRTIO, or we'd probably have leaked them.) >> we would >> apparently try to create the implicit drive, but in fact not do so >> because vl.c had already done device creation long before. I've included >> a patch that makes it more explicit that hotplug does not get you the >> magic implicit devices. > > PATCH 2/4. Haven't thought through its implications. It's not supposed to have implications, it's supposed to be a "no behaviour change" patch :-) Without the s390 special casing patches 2 and 3 collapse down into a single patch, incidentally. -- PMM
[Qemu-block] [PATCH v2 2/3] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
If a user requests an IF_VIRTIO drive on the command line, don't
create the implicit PCI virtio device immediately, but wait until
the rest of the command line has been processed and only create the
device if the drive would otherwise be orphaned. This means that
if the user said drive,id=something,... -device drive=something,..,.
we'll allow the drive to be connected to the user's specified
device rather than stealing it to connect to the implicit virtio
device.
This change does reorder device creation (which will mean a
migration break, and guest visible changes like which PCI
slot implicitly created devices appear in). We can do this
because no machine currently specifies a block_default_type of
IF_VIRTIO except for the S390 machines, and those machines
do not currently support cross-version migration anyway.
Although at first glance it looks like this commit is changing
the behaviour for hotplugged devices, it is not: although
hotplugged devices used to call the code to create an implicit
virtio device, this had no effect because the code in vl.c to
create devices from the devopts list had already run once and
would not be run again.
Signed-off-by: Peter Maydell
---
blockdev.c | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 9cf6123..68cdbfe 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -46,6 +46,7 @@
#include "qmp-commands.h"
#include "trace.h"
#include "sysemu/arch_init.h"
+#include "monitor/qdev.h"
static const char *const if_name[IF_COUNT] = {
[IF_NONE] = "none",
@@ -231,6 +232,15 @@ static void create_implicit_virtio_device(const char
*driveid,
if (devaddr) {
qemu_opt_set(devopts, "addr", devaddr, &error_abort);
}
+
+/* We're called after vl.c has processed the -device options,
+ * so we need to create the device ourselves now.
+ */
+DeviceState *dev = qdev_device_add(devopts);
+if (!dev) {
+exit(1);
+}
+object_unref(OBJECT(dev));
}
bool drive_check_orphaned(void)
@@ -245,6 +255,14 @@ bool drive_check_orphaned(void)
/* Unless this is a default drive, this may be an oversight. */
if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
dinfo->type != IF_NONE) {
+if (dinfo->type == IF_VIRTIO) {
+/* An orphaned virtio drive might be waiting for us to
+ * create the implicit device for it.
+ */
+create_implicit_virtio_device(blk_name(blk), dinfo->devaddr);
+continue;
+}
+
fprintf(stderr, "Warning: Orphaned drive without device: "
"id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
blk_name(blk), blk_bs(blk)->filename, if_name[dinfo->type],
@@ -949,10 +967,6 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
goto fail;
}
-if (type == IF_VIRTIO) {
-create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
-}
-
filename = qemu_opt_get(legacy_opts, "file");
/* Check werror/rerror compatibility with if=... */
--
1.9.1
[Qemu-block] [PATCH v2 0/3] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
This is a respin of the patchset I sent out last week; the only change v1->v2 is to drop the special casing of S390, as Christian confirmed it isn't needed. This collapses the old patches 2/3 into one. I'm going to use this cover letter as an opportunity to (a) hopefully be a bit clearer about the intention and (b) try to summarise the current behaviour and possible improvements. Patchset Rationale == blockdev.c will create implicit virtio-blk-* devices for IF_VIRTIO drives. I want to turn this on for the ARM virt board (now it has PCI), so that users can use shorter and more comprehensible command lines, including the "-hda" very-short convenience options. In particular I'm getting justified complaints that the virt board's command line requirements for disk specification are complicated and different from x86. (The virt board currently ends up with the default default drive type, which is IDE, which is totally useless as there is no IDE controller.) Patch 3 is the one-liner to change the default-drive-type. Unfortunately just doing that alone will break commandlines that currently work and are I think relatively common with people using this board, like: -drive file=arm-wheezy.qcow2,id=foo -device virtio-blk-pci,drive=foo (Note the lack of an if= specifier.) This works in current master because nothing complains about the fact you've (implicitly) specified an if=ide drive on a machine with no IDE drive, or about the fact you've just plugged an if=ide drive into a virtio-blk device. I *really* want to be able to set the default drive type to IF_VIRTIO; if I have to break command lines like the above in order to do so, I can put up with that, but I'm hoping we can do better than that. Current situation = At the moment we somewhat under-check uses of the if=foo option to drives: * we do check that drives are not completely orphaned * we do insist that drives aren't connected twice * we don't directly check that an if=ide drive is actually plugged into an IDE controller (ditto SCSI, virtio, etc etc) The "no double connection" rule means we will indirectly notice some if= mismatches: if the machine has a bus of the correct type and claims an if=foo drive, then this will get a connected-twice error (from the automatic-claiming, and from the user-specified commandline connection). However if the machine doesn't have a bus of the relevant type and so doesn't auto-claim the drive, we won't detect any problem. So x86_64 is inconsistent: qemu-system-x86_64 -nodefaults -display none -drive if=scsi,file=tmp.qcow2,id=foo -device ide-hd,drive=foo fails, but qemu-system-x86_64 -nodefaults -display none -drive if=sd,file=tmp.qcow2,id=foo -device ide-hd,drive=foo does not, and will connect an "if=sd" drive to the IDE controller. Where we might like to be = Some possibilities, presented without regard to back-compat: Option A: 1) explicit if=foo drive manually wired up to a device is an error (and always diagnosed as such, not indirectly via it being double-used) 2) explicit if=none drive is never auto-claimed 3a) drive with no if= is exactly as if the user explicitly stated if= and must be auto-claimed [This is almost what we have right now, except we do 1 only sporadically and accidentally, and so some users are probably using option combos we'd like to have be illegal] Option B: 1) and 2) as above, but instead of 3a): 3b) drive with no if=... means "if user wired it up to something on the command line, honour that; otherwise treat as if= and auto-claim [This tries to make at least some of the option-combos do the plausibly right thing. This series is trying to add that for virtio, but other if= types are probably harder to deal with. I think the semantics here are nice but I'm struggling to see how to implement it.] Option C: Any other reasonable suggestions? If we can decide what we'd like the semantics to be (ideally with some idea of how to implement them ;-)) I can have a stab at writing some patches. I do definitely want to enable short-options for virt for 2.4... thanks -- PMM Peter Maydell (3): blockdev: Factor out create_implicit_virtio_device blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives hw/arm/virt: Make block devices default to virtio blockdev.c| 55 +++ hw/arm/virt.c | 1 + 2 files changed, 40 insertions(+), 16 deletions(-) -- 1.9.1
[Qemu-block] [PATCH v2 1/3] blockdev: Factor out create_implicit_virtio_device
Factor out the code which creates the implicit virtio device
for IF_VIRTIO drives, so we can call it from drive_check_orphaned().
Signed-off-by: Peter Maydell
---
blockdev.c | 35 ++-
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index de94a8b..9cf6123 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -212,6 +212,27 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int
unit)
return NULL;
}
+/* Create the virtio device whose existence is implied by the
+ * creation of a block device with if=virtio.
+ */
+static void create_implicit_virtio_device(const char *driveid,
+ const char *devaddr)
+{
+QemuOpts *devopts;
+
+devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ &error_abort);
+if (arch_type == QEMU_ARCH_S390X) {
+qemu_opt_set(devopts, "driver", "virtio-blk-s390", &error_abort);
+} else {
+qemu_opt_set(devopts, "driver", "virtio-blk-pci", &error_abort);
+}
+qemu_opt_set(devopts, "drive", driveid, &error_abort);
+if (devaddr) {
+qemu_opt_set(devopts, "addr", devaddr, &error_abort);
+}
+}
+
bool drive_check_orphaned(void)
{
BlockBackend *blk;
@@ -929,19 +950,7 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
}
if (type == IF_VIRTIO) {
-QemuOpts *devopts;
-devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
- &error_abort);
-if (arch_type == QEMU_ARCH_S390X) {
-qemu_opt_set(devopts, "driver", "virtio-blk-s390", &error_abort);
-} else {
-qemu_opt_set(devopts, "driver", "virtio-blk-pci", &error_abort);
-}
-qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
- &error_abort);
-if (devaddr) {
-qemu_opt_set(devopts, "addr", devaddr, &error_abort);
-}
+create_implicit_virtio_device(qdict_get_str(bs_opts, "id"), devaddr);
}
filename = qemu_opt_get(legacy_opts, "file");
--
1.9.1
[Qemu-block] [PATCH v2 3/3] hw/arm/virt: Make block devices default to virtio
Now we have virtio-pci, we can make the virt board's default block
device type be IF_VIRTIO. This allows users to use simplified
command lines that don't have to explicitly create virtio-pci-blk
devices; the -hda &c very short options now also work.
Signed-off-by: Peter Maydell
---
hw/arm/virt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1b1cc71..1cb56c8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -961,6 +961,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
mc->init = machvirt_init;
mc->max_cpus = 8;
mc->has_dynamic_sysbus = true;
+mc->block_default_type = IF_VIRTIO;
}
static const TypeInfo machvirt_info = {
--
1.9.1
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/3] blockdev: Defer creation of implicit PCI devices for IF_VIRTIO drives
On 9 June 2015 at 18:48, Peter Maydell wrote: > Patch 3 is the one-liner to change the default-drive-type. Unfortunately > just doing that alone will break commandlines that currently work The other problem with taking patch 3 alone is that it completely breaks everything, because: * by default we create a "cdrom" drive whose type is and which has no media inserted * the virtio-blk device barfs if you give it a drive with no media: "Device needs media, but drive is empty" The S390 systems get around this by specifying no_cdrom = 1, but it doesn't seem terribly satisfactory that this has to be manually done by any machine with a virtio default drive type... -- PMM
