Re: [Qemu-devel] [PATCH v4 11/23] block: Rename BlockDriverAIOCB* to BlockAIOCB*
Max Reitz writes: > On 30.09.2014 21:25, Markus Armbruster wrote: >> I'll use BlockDriverAIOCB with block backends shortly, and the name is >> going to fit badly there. It's a block layer thing anyway, not just a >> block driver thing. >> >> Signed-off-by: Markus Armbruster >> --- >> block-migration.c | 2 +- >> block.c | 124 ++-- >> block/archipelago.c | 10 ++-- >> block/blkdebug.c| 10 ++-- >> block/blkverify.c | 8 +-- >> block/curl.c| 4 +- >> block/iscsi.c | 6 +-- >> block/linux-aio.c | 6 +-- >> block/null.c| 10 ++-- >> block/qed.c | 8 +-- >> block/qed.h | 2 +- >> block/quorum.c | 36 ++--- >> block/raw-aio.h | 4 +- >> block/raw-posix.c | 16 +++--- >> block/raw-win32.c | 8 +-- >> block/raw_bsd.c | 8 +-- >> block/rbd.c | 12 ++--- >> block/sheepdog.c| 4 +- >> block/win32-aio.c | 4 +- >> dma-helpers.c | 12 ++--- >> hw/block/nvme.h | 2 +- >> hw/ide/ahci.h | 2 +- >> hw/ide/core.c | 8 +-- >> hw/ide/internal.h | 6 +-- >> hw/ppc/mac.h| 2 +- >> hw/scsi/scsi-generic.c | 2 +- >> include/block/aio.h | 8 +-- >> include/block/block.h | 20 +++ >> include/block/block_int.h | 10 ++-- >> include/block/thread-pool.h | 2 +- >> include/hw/scsi/scsi.h | 2 +- >> include/sysemu/dma.h| 26 +- >> tests/test-thread-pool.c| 2 +- >> thread-pool.c | 8 +-- >> 34 files changed, 197 insertions(+), 197 deletions(-) >> > > [snip] > >> diff --git a/block/archipelago.c b/block/archipelago.c >> index 73d91a4..edbfbb5 100644 >> --- a/block/archipelago.c >> +++ b/block/archipelago.c >> @@ -86,7 +86,7 @@ typedef enum { >> } ARCHIPCmd; >> typedef struct ArchipelagoAIOCB { >> -BlockDriverAIOCB common; >> +BlockAIOCB common; >> QEMUBH *bh; >> struct BDRVArchipelagoState *s; >> QEMUIOVector *qiov; >> @@ -856,7 +856,7 @@ err_exit: >> return ret; >> } >> -static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState >> *bs, >> +static BlockAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs, >>int64_t sector_num, >>QEMUIOVector *qiov, >>int nb_sectors, > > This breaks the alignment. Fixing... (the others, too) > [snip] > >> diff --git a/block/blkverify.c b/block/blkverify.c >> index 7d64a23..a29ed05 100644 >> --- a/block/blkverify.c >> +++ b/block/blkverify.c > > [snip] > >> -static BlockDriverAIOCB *blkverify_aio_flush(BlockDriverState *bs, >> +static BlockAIOCB *blkverify_aio_flush(BlockDriverState *bs, >>BlockDriverCompletionFunc *cb, >>void *opaque) >> { > > Breaks the alignment. > > [snip] > >> diff --git a/block/null.c b/block/null.c >> index 8284419..b353a73 100644 >> --- a/block/null.c >> +++ b/block/null.c > > [snip] > >> @@ -94,7 +94,7 @@ static void null_bh_cb(void *opaque) >> qemu_aio_unref(acb); >> } >> -static inline BlockDriverAIOCB *null_aio_common(BlockDriverState >> *bs, >> +static inline BlockAIOCB *null_aio_common(BlockDriverState *bs, >> BlockDriverCompletionFunc >> *cb, >> void *opaque) >> { > > Alignment again (applies to all hunks in block/null.c touching > function headers). > > [snip] > >> diff --git a/block/qed.c b/block/qed.c >> index 7f03c26..0382228 100644 >> --- a/block/qed.c >> +++ b/block/qed.c >> @@ -1365,7 +1365,7 @@ static void qed_aio_next_io(void *opaque, int ret) >> io_fn, acb); >> } >> -static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs, >> +static BlockAIOCB *qed_aio_setup(BlockDriverState *bs, >> int64_t sector_num, >> QEMUIOVector *qiov, int nb_sectors, >> BlockDriverCompletionFunc *cb, > > Alignment, for all function header hunks in block/qed.c. > > [snip] > >> diff --git a/block/rbd.c b/block/rbd.c >> index 96947e3..f44a093 100644 >> --- a/block/rbd.c >> +++ b/block/rbd.c > > [snip] > >> @@ -589,7 +589,7 @@ static int rbd_aio_flush_wrapper(rbd_image_t image, >> #endif >> } >> -static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs, >> +static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, >> int64_t sector_num, >> QEMU
Re: [Qemu-devel] [PATCH v4 12/23] block: Rename BlockDriverCompletionFunc to BlockCompletionFunc
Max Reitz writes: > On 30.09.2014 21:25, Markus Armbruster wrote: >> I'll use it with block backends shortly, and the name is going to fit >> badly there. It's a block layer thing anyway, not just a block driver >> thing. >> >> Signed-off-by: Markus Armbruster >> --- >> block.c | 30 +++--- >> block/archipelago.c | 8 >> block/backup.c | 2 +- >> block/blkdebug.c| 8 >> block/blkverify.c | 8 >> block/commit.c | 2 +- >> block/curl.c| 2 +- >> block/iscsi.c | 2 +- >> block/linux-aio.c | 2 +- >> block/mirror.c | 6 +++--- >> block/null.c| 8 >> block/qed-gencb.c | 4 ++-- >> block/qed-table.c | 10 +- >> block/qed.c | 18 +- >> block/qed.h | 10 +- >> block/quorum.c | 6 +++--- >> block/raw-aio.h | 4 ++-- >> block/raw-posix.c | 16 >> block/raw-win32.c | 8 >> block/raw_bsd.c | 2 +- >> block/rbd.c | 10 +- >> block/stream.c | 2 +- >> block/win32-aio.c | 2 +- >> blockjob.c | 4 ++-- >> dma-helpers.c | 2 +- >> hw/ide/ahci.c | 2 +- >> hw/ide/core.c | 4 ++-- >> hw/ide/internal.h | 6 +++--- >> hw/ide/macio.c | 2 +- >> hw/ide/pci.c| 2 +- >> hw/ide/pci.h| 2 +- >> hw/scsi/scsi-generic.c | 2 +- >> include/block/aio.h | 6 +++--- >> include/block/block.h | 14 +++--- >> include/block/block_int.h | 20 ++-- >> include/block/blockjob.h| 4 ++-- >> include/block/thread-pool.h | 2 +- >> include/monitor/monitor.h | 4 ++-- >> include/sysemu/dma.h| 8 >> monitor.c | 6 +++--- >> thread-pool.c | 2 +- >> 41 files changed, 131 insertions(+), 131 deletions(-) > > I requested some trivial changes to patch 11 and rebasing this patch > unto it will result in some trivial "functional" changes, if you heed > my requests. Those should only be alignment changes, however (even the > hunk in scsi-generic.c). In addition, docs/blkdebug.txt mentions > BlockDriverCompletionFunc. Thus, like patch 11, this patch should > replace that occurrence as well. > > With these trivial changes (alignment changes due to rebasing on a > patch 11 changed according to my proposals and the replacement in > docs/blkdebug.txt): > > Reviewed-by: Max Reitz Done, thanks!
Re: [Qemu-devel] [PATCH v4 15/23] hw: Convert from BlockDriverState to BlockBackend, mostly
Max Reitz writes: > On 30.09.2014 21:25, Markus Armbruster wrote: >> Device models should access their block backends only through the >> block-backend.h API. Convert them, and drop direct includes of >> inappropriate headers. >> >> Just four uses of BlockDriverState are left: >> >> * The Xen paravirtual block device backend (xen_disk.c) opens images >>itself when set up via xenbus, bypassing blockdev.c. I figure it >>should go through qmp_blockdev_add() instead. >> >> * Device model "usb-storage" prompts for keys. No other device model >>does, and this one probably shouldn't do it, either. >> >> * ide_issue_trim_cb() uses bdrv_aio_discard() instead of >>blk_aio_discard() because it fishes its backend out of a BlockAIOCB, >>which has only the BlockDriverState. >> >> * PC87312State has an unused BlockDriverState[] member. >> >> The next two commits take care of the latter two. >> >> Signed-off-by: Markus Armbruster >> --- >> block/block-backend.c| 262 >> +++ >> blockdev.c | 19 +-- >> dma-helpers.c| 59 +++ >> hw/arm/collie.c | 5 +- >> hw/arm/gumstix.c | 5 +- >> hw/arm/highbank.c| 2 +- >> hw/arm/mainstone.c | 2 +- >> hw/arm/musicpal.c| 10 +- >> hw/arm/nseries.c | 3 +- >> hw/arm/omap1.c | 2 +- >> hw/arm/omap2.c | 2 +- >> hw/arm/omap_sx1.c| 5 +- >> hw/arm/pxa2xx.c | 4 +- >> hw/arm/realview.c| 2 +- >> hw/arm/spitz.c | 4 +- >> hw/arm/tosa.c| 3 +- >> hw/arm/versatilepb.c | 3 +- >> hw/arm/vexpress.c| 3 +- >> hw/arm/virt.c| 2 +- >> hw/arm/xilinx_zynq.c | 3 +- >> hw/arm/z2.c | 3 +- >> hw/block/block.c | 7 +- >> hw/block/dataplane/virtio-blk.c | 24 +-- >> hw/block/fdc.c | 78 + >> hw/block/hd-geometry.c | 24 +-- >> hw/block/m25p80.c| 28 ++-- >> hw/block/nand.c | 50 +++--- >> hw/block/nvme.c | 19 +-- >> hw/block/onenand.c | 67 >> hw/block/pflash_cfi01.c | 24 +-- >> hw/block/pflash_cfi02.c | 24 +-- >> hw/block/virtio-blk.c| 95 +-- >> hw/block/xen_disk.c | 83 +- >> hw/core/qdev-properties-system.c | 26 +-- >> hw/core/qdev-properties.c| 2 +- >> hw/cris/axis_dev88.c | 3 +- >> hw/display/tc6393xb.c| 2 +- >> hw/i386/pc.c | 2 +- >> hw/i386/pc_piix.c| 2 +- >> hw/i386/pc_sysfw.c | 9 +- >> hw/i386/xen/xen_platform.c | 5 +- >> hw/ide/ahci.c| 31 ++-- >> hw/ide/atapi.c | 33 ++-- >> hw/ide/cmd646.c | 2 +- >> hw/ide/core.c| 184 -- >> hw/ide/ich.c | 2 +- >> hw/ide/internal.h| 6 +- >> hw/ide/isa.c | 2 +- >> hw/ide/macio.c | 50 +++--- >> hw/ide/microdrive.c | 4 +- >> hw/ide/mmio.c| 2 +- >> hw/ide/pci.c | 4 +- >> hw/ide/piix.c| 9 +- >> hw/ide/qdev.c| 11 +- >> hw/ide/via.c | 2 +- >> hw/isa/pc87312.c | 4 +- >> hw/lm32/lm32_boards.c| 5 +- >> hw/lm32/milkymist.c | 3 +- >> hw/microblaze/petalogix_ml605_mmu.c | 3 +- >> hw/microblaze/petalogix_s3adsp1800_mmu.c | 3 +- >> hw/mips/mips_fulong2e.c | 2 +- >> hw/mips/mips_jazz.c | 2 +- >> hw/mips/mips_malta.c | 6 +- >> hw/mips/mips_r4k.c | 3 +- >> hw/nvram/spapr_nvram.c | 17 +- >> hw/pci/pci-hotplug-old.c | 5 +- >> hw/ppc/mac_newworld.c| 2 +- >> hw/ppc/mac_oldworld.c| 2 +- >> hw/ppc/ppc405_boards.c | 26 +-- >> hw/ppc/prep.c| 2 +- >> hw/ppc/spapr.c | 4 +- >> hw/ppc/virtex_ml50
[Qemu-devel] [PATCH] configure: Filter out system includes for pixman
Other packages may provide includes for pixman as well if the host has a devel package installed. Remove them from QEMU_CFLAGS before adding our version to unsure that the right headers are used. Signed-off-by: Jan Kiszka --- configure | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 9ac2600..98c7cc8 100755 --- a/configure +++ b/configure @@ -4211,8 +4211,9 @@ EOF fi fi -# add pixman flags after all config tests are done -QEMU_CFLAGS="$QEMU_CFLAGS $pixman_cflags $fdt_cflags" +# add pixman flags after all config tests are done, +# filtering out conflicting includes paths +QEMU_CFLAGS=`echo "$QEMU_CFLAGS" | sed "s/-I[^ ]*pixman-1//"`" $pixman_cflags $fdt_cflags" libs_softmmu="$libs_softmmu $pixman_libs" echo "Install prefix$prefix" -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [RFC patch 0/6] vfio based pci pass-through for qemu/KVM on s390
On Wed, Oct 01, 2014 at 11:26:51AM -0600, Alex Williamson wrote: > On Wed, 2014-10-01 at 11:11 +0200, Frank Blaschka wrote: > > On Fri, Sep 26, 2014 at 01:59:40PM -0600, Alex Williamson wrote: > > > On Fri, 2014-09-26 at 08:45 +0200, Frank Blaschka wrote: > > > > On Wed, Sep 24, 2014 at 10:05:57AM -0600, Alex Williamson wrote: > > > > > On Wed, 2014-09-24 at 10:47 +0200, Frank Blaschka wrote: > > > > > > On Mon, Sep 22, 2014 at 02:47:31PM -0600, Alex Williamson wrote: > > > > > > > On Fri, 2014-09-19 at 13:54 +0200, frank.blasc...@de.ibm.com > > > > > > > wrote: > > > > > > > > This set of patches implements a vfio based solution for pci > > > > > > > > pass-through on the s390 platform. The kernel stuff is pretty > > > > > > > > much straight forward, but qemu needs more work. > > > > > > > > > > > > > > > > Most interesting patch is: > > > > > > > > vfio: make vfio run on s390 platform > > > > > > > > > > > > > > > > I hope Alex & Alex can give me some guidance how to do the > > > > > > > > changes > > > > > > > > in an appropriate way. After creating a separate iommmu address > > > > > > > > space > > > > > > > > for each attached PCI device I can successfully run the vfio > > > > > > > > type1 > > > > > > > > iommu. So If we could extend type1 not registering all guest > > > > > > > > memory > > > > > > > > (see patch) I think we do not need a special vfio iommu for s390 > > > > > > > > for the moment. > > > > > > > > > > > > > > > > The patches implement the base pass-through support. s390 > > > > > > > > specific > > > > > > > > virtualization functions are currently not included. This would > > > > > > > > be a second step after the base support is done. > > > > > > > > > > > > > > > > kernel patches apply to linux-kvm-next > > > > > > > > > > > > > > > > KVM: s390: Enable PCI instructions > > > > > > > > iommu: add iommu for s390 platform > > > > > > > > vfio: make vfio build on s390 > > > > > > > > > > > > > > > > qemu patches apply to qemu-master > > > > > > > > > > > > > > > > s390: Add PCI bus support > > > > > > > > s390: implement pci instruction > > > > > > > > vfio: make vfio run on s390 platform > > > > > > > > > > > > > > > > Thx for feedback and review comments > > > > > > > > > > > > > > Sending patches as attachments makes it difficult to comment > > > > > > > inline. > > > > > > > > > > > > > Sorry, don't understand this. I sent every patch as separate email > > > > > > so > > > > > > you can comment directly on the patch. What do you prefer? > > > > > > > > > > The patches in each email are showing up as attachments in my mail > > > > > client. Is it just me? > > > > > > > > > > > > 2/6 > > > > > > > - careful of the namespace as you're changing functions from > > > > > > > static and > > > > > > > exporting them > > > > > > > - doesn't seem like functions need to be exported, just > > > > > > > non-static to > > > > > > > call from s390-iommu.c > > > > > > > > > > > > > Ok, will change this. > > > > > > > > > > > > > 6/6 > > > > > > > - We shouldn't need to globally disable mmap, each VFIO region > > > > > > > reports > > > > > > > whether it supports mmap and vfio-pci on s390 should indicate > > > > > > > mmap is > > > > > > > not supported on the platform. > > > > > > Yes, this is even better to let the kernel announce a BAR can not be > > > > > > mmap'ed. Checking the kernel code I realized the BARs are valid for > > > > > > mmap'ing but the s390 platform does simply not allow this. So I > > > > > > feal we > > > > > > have to introduce a platform switch in kernel. How about this ... > > > > > > > > > > > > --- a/drivers/vfio/pci/vfio_pci.c > > > > > > +++ b/drivers/vfio/pci/vfio_pci.c > > > > > > @@ -377,9 +377,11 @@ static long vfio_pci_ioctl(void *device_ > > > > > > > > > > > > info.flags = VFIO_REGION_INFO_FLAG_READ | > > > > > > VFIO_REGION_INFO_FLAG_WRITE; > > > > > > +#ifndef CONFIG_S390 > > > > > > if (pci_resource_flags(pdev, info.index) & > > > > > > IORESOURCE_MEM && info.size >= > > > > > > PAGE_SIZE) > > > > > > info.flags |= > > > > > > VFIO_REGION_INFO_FLAG_MMAP; > > > > > > +#endif > > > > > > break; > > > > > > case VFIO_PCI_ROM_REGION_INDEX: > > > > > > { > > > > > > > > > > Maybe pull it out into a function. Also, is there some capability or > > > > > feature we can test rather than just the architecture? I'd prefer it > > > > > to > > > > > be excluded because of a platform feature that prevents it rather than > > > > > the overall architecture itself. > > > > > > > > > > > > > Ok, understand this. There is no capability of feature so I will go with > > > > the function. > > > > > > > > > > > - INTx should be done the same way, the interrupt index for INTx > > > > > > > should > > > > > > > report 0 count. The curren
Re: [Qemu-devel] [PATCH 1/3] pc: Fix disabling of vapic for compat PC models
On 2014-09-02 17:11, Michael Roth wrote: > Quoting Markus Armbruster (2014-07-30 06:19:36) >> Paolo Bonzini writes: >> >>> Il 30/07/2014 10:57, Michael S. Tsirkin ha scritto: On Wed, Jul 30, 2014 at 09:01:59AM +0200, Jan Kiszka wrote: > We used to be able to address both the QEMU and the KVM APIC via "apic". > This doesn't work anymore. So we need to use their parent class to turn > off the vapic on machines that should not expose them. > > Signed-off-by: Jan Kiszka OK so this is intended for 2.2? >> >> If yes, we should cc: qemu-stable. > > Ping for stable 2.1.1, freeze is on Wednesday Lost track of this: was I supposed to provide anything different, or did this just fall under the table? Jan > >> In that case, how about creating a macro with type name, and using that? This way things don't break if we rename something again. >>> >>> Don't we have warnings for that now? >> >> Warnings don't help much in cases like this: "apic" still exists and has >> the property, it's just not the device we want. Macros aren't >> foolproof, either. >> > --- > hw/i386/pc_piix.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 9694f88..73ba77d 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -645,7 +645,7 @@ static QEMUMachine pc_machine_v1_1 = { > .property = "class",\ > .value= stringify(PCI_CLASS_MEMORY_RAM),\ > },{\ > -.driver = "apic",\ > +.driver = "apic-common",\ > .property = "vapic",\ > .value= "off",\ > },{\ > -- > 1.8.1.1.298.ge7eed54 >> >> You could use TYPE_APIC_COMMON here. Including >> "hw/i386/apic_internal.h" for it would be not so nice, though. > -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] configure: Filter out system includes for pixman
On Do, 2014-10-02 at 09:14 +0200, Jan Kiszka wrote: > Other packages may provide includes for pixman as well if the host has a > devel package installed. Remove them from QEMU_CFLAGS before adding our > version to unsure that the right headers are used. Hmm, how does that happen? Shouldn't qemu use the system pixman if present? Well, maybe not in case it is too old ... I think we should just reverse the ordering, so our pixman submodule comes first in the include path list. And it is probably a good idea to do the same for fdt. cheers, Gerd
[Qemu-devel] [PULL 00/10] vga: cleanups, prepare for endianness switching
Hi, vga cleanup patches finally on the path upstream ;) please pull, Gerd The following changes since commit 29429c7244c73eefada3d0ec6dd30c5698782d08: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140929' into staging (2014-09-30 11:02:06 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-vga-20141002-1 for you to fetch changes up to c3b10605147f9113b8b157d7226d3e215184bc0e: vga: Add endian to vmstate (2014-09-30 13:34:09 +0200) vga: cleanups, prepare for endianness switching Benjamin Herrenschmidt (10): vga: Start cutting out non-32bpp conversion support vga: Remove remainder of old conversion cruft vga: Separate LE and BE conversion functions vga: Remove rgb_to_pixel indirection vga: Simplify vga_draw_blank() a bit cirrus: Remove non-32bpp cursor drawing vga: Remove some "should be done in BIOS" comments vga: Rename vga_template.h to vga-helpers.h vga: Make fb endian a common state variable vga: Add endian to vmstate hw/display/cirrus_vga.c | 79 +++--- hw/display/cirrus_vga_template.h | 102 --- hw/display/{vga_template.h => vga-helpers.h} | 318 +++--- hw/display/vga.c | 382 +++ hw/display/vga_int.h | 5 +- 5 files changed, 296 insertions(+), 590 deletions(-) delete mode 100644 hw/display/cirrus_vga_template.h rename hw/display/{vga_template.h => vga-helpers.h} (52%)
[Qemu-devel] [PULL 04/10] vga: Remove rgb_to_pixel indirection
From: Benjamin Herrenschmidt We always use rgb_to_pixel32 nowadays. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/cirrus_vga.c | 15 +-- hw/display/vga.c| 31 ++- hw/display/vga_int.h| 2 -- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index db330e9..6f8d149 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -29,6 +29,7 @@ #include "hw/hw.h" #include "hw/pci/pci.h" #include "ui/console.h" +#include "ui/pixel_ops.h" #include "vga_int.h" #include "hw/loader.h" @@ -2212,6 +2213,8 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) } else { src += (s->vga.sr[0x13] & 0x3f) * 256; src += (scr_y - s->hw_cursor_y) * 4; + + poffset = 128; content = ((uint32_t *)src)[0] | ((uint32_t *)(src + 128))[0]; @@ -2229,12 +2232,12 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) x2 = s->vga.last_scr_width; w = x2 - x1; palette = s->cirrus_hidden_palette; -color0 = s->vga.rgb_to_pixel(c6_to_8(palette[0x0 * 3]), - c6_to_8(palette[0x0 * 3 + 1]), - c6_to_8(palette[0x0 * 3 + 2])); -color1 = s->vga.rgb_to_pixel(c6_to_8(palette[0xf * 3]), - c6_to_8(palette[0xf * 3 + 1]), - c6_to_8(palette[0xf * 3 + 2])); +color0 = rgb_to_pixel32(c6_to_8(palette[0x0 * 3]), +c6_to_8(palette[0x0 * 3 + 1]), +c6_to_8(palette[0x0 * 3 + 2])); +color1 = rgb_to_pixel32(c6_to_8(palette[0xf * 3]), +c6_to_8(palette[0xf * 3 + 1]), +c6_to_8(palette[0xf * 3 + 2])); bpp = surface_bytes_per_pixel(surface); d1 += x1 * bpp; switch (surface_bits_per_pixel(surface)) { diff --git a/hw/display/vga.c b/hw/display/vga.c index 451c354..9d06691 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1011,13 +1011,6 @@ typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, #include "vga_template.h" -static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel32(r, g, b); -return col; -} - /* return true if the palette was modified */ static int update_palette16(VGACommonState *s) { @@ -1034,9 +1027,9 @@ static int update_palette16(VGACommonState *s) v = ((s->ar[VGA_ATC_COLOR_PAGE] & 0xc) << 4) | (v & 0x3f); } v = v * 3; -col = s->rgb_to_pixel(c6_to_8(s->palette[v]), - c6_to_8(s->palette[v + 1]), - c6_to_8(s->palette[v + 2])); +col = rgb_to_pixel32(c6_to_8(s->palette[v]), + c6_to_8(s->palette[v + 1]), + c6_to_8(s->palette[v + 2])); if (col != palette[i]) { full_update = 1; palette[i] = col; @@ -1056,13 +1049,13 @@ static int update_palette256(VGACommonState *s) v = 0; for(i = 0; i < 256; i++) { if (s->dac_8bit) { - col = s->rgb_to_pixel(s->palette[v], -s->palette[v + 1], -s->palette[v + 2]); +col = rgb_to_pixel32(s->palette[v], + s->palette[v + 1], + s->palette[v + 2]); } else { - col = s->rgb_to_pixel(c6_to_8(s->palette[v]), -c6_to_8(s->palette[v + 1]), -c6_to_8(s->palette[v + 2])); +col = rgb_to_pixel32(c6_to_8(s->palette[v]), + c6_to_8(s->palette[v + 1]), + c6_to_8(s->palette[v + 2])); } if (col != palette[i]) { full_update = 1; @@ -1245,7 +1238,6 @@ static void vga_draw_text(VGACommonState *s, int full_update) s->last_cw = cw; full_update = 1; } -s->rgb_to_pixel = rgb_to_pixel32_dup; full_update |= update_palette16(s); palette = s->last_palette; x_incr = cw * surface_bytes_per_pixel(surface); @@ -1553,8 +1545,6 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) dpy_gfx_replace_surface(s->con, surface); } -s->rgb_to_pixel = rgb_to_pixel32_dup; - if (shift_control == 0) { full_update |= update_palette16(s); if (s->sr[VGA_SEQ_CLOCK_MODE] & 8) { @@ -1700,9 +1690,8 @@ static void vga_draw_blank(VGACommonState *s, int full_update) if (s->last_scr_width <= 0 || s->last_scr_height <= 0) return; -s->rgb_to_pixel = rgb_to_pixel32_dup; if (surface_bits_per_pixel(surface) == 8) { -val = s->rgb_to
[Qemu-devel] [PULL 03/10] vga: Separate LE and BE conversion functions
From: Benjamin Herrenschmidt Provide different functions for converting from an LE vs a BE framebuffer. We cannot rely on the simple cases always being shared surfaces since cirrus will need to always shadow for cursor emulation, so we need the full set of functions to be able to later handle runtime switching. Signed-off-by: Benjamin Herrenschmidt \ Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 43 +++--- hw/display/vga_template.h | 109 +++--- 2 files changed, 112 insertions(+), 40 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 721309f..451c354 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1374,10 +1374,14 @@ enum { VGA_DRAW_LINE4D2, VGA_DRAW_LINE8D2, VGA_DRAW_LINE8, -VGA_DRAW_LINE15, -VGA_DRAW_LINE16, -VGA_DRAW_LINE24, -VGA_DRAW_LINE32, +VGA_DRAW_LINE15_LE, +VGA_DRAW_LINE16_LE, +VGA_DRAW_LINE24_LE, +VGA_DRAW_LINE32_LE, +VGA_DRAW_LINE15_BE, +VGA_DRAW_LINE16_BE, +VGA_DRAW_LINE24_BE, +VGA_DRAW_LINE32_BE, VGA_DRAW_LINE_NB, }; @@ -1388,10 +1392,14 @@ static vga_draw_line_func * const vga_draw_line_table[VGA_DRAW_LINE_NB] = { vga_draw_line4d2, vga_draw_line8d2, vga_draw_line8, -vga_draw_line15, -vga_draw_line16, -vga_draw_line24, -vga_draw_line32, +vga_draw_line15_le, +vga_draw_line16_le, +vga_draw_line24_le, +vga_draw_line32_le, +vga_draw_line15_be, +vga_draw_line16_be, +vga_draw_line24_be, +vga_draw_line32_be, }; static int vga_get_bpp(VGACommonState *s) @@ -1464,10 +1472,15 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) uint8_t *d; uint32_t v, addr1, addr; vga_draw_line_func *vga_draw_line; -#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) -static const bool byteswap = false; +#if defined(TARGET_WORDS_BIGENDIAN) +static const bool big_endian_fb = true; #else -static const bool byteswap = true; +static const bool big_endian_fb = false; +#endif +#if defined(HOST_WORDS_BIGENDIAN) +static const bool byteswap = !big_endian_fb; +#else +static const bool byteswap = big_endian_fb; #endif full_update |= update_basic_params(s); @@ -1572,19 +1585,19 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) bits = 8; break; case 15: -v = VGA_DRAW_LINE15; +v = big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; bits = 16; break; case 16: -v = VGA_DRAW_LINE16; +v = big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; bits = 16; break; case 24: -v = VGA_DRAW_LINE24; +v = big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; bits = 24; break; case 32: -v = VGA_DRAW_LINE32; +v = big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; bits = 32; break; } diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h index 0660b52..94f6de2 100644 --- a/hw/display/vga_template.h +++ b/hw/display/vga_template.h @@ -278,21 +278,36 @@ static void vga_draw_line8(VGACommonState *s1, uint8_t *d, } } - -/* XXX: optimize */ - /* * 15 bit color */ -static void vga_draw_line15(VGACommonState *s1, uint8_t *d, -const uint8_t *s, int width) +static void vga_draw_line15_le(VGACommonState *s1, uint8_t *d, + const uint8_t *s, int width) { int w; uint32_t v, r, g, b; w = width; do { -v = lduw_p((void *)s); +v = lduw_le_p((void *)s); +r = (v >> 7) & 0xf8; +g = (v >> 2) & 0xf8; +b = (v << 3) & 0xf8; +((uint32_t *)d)[0] = rgb_to_pixel32(r, g, b); +s += 2; +d += 4; +} while (--w != 0); +} + +static void vga_draw_line15_be(VGACommonState *s1, uint8_t *d, + const uint8_t *s, int width) +{ +int w; +uint32_t v, r, g, b; + +w = width; +do { +v = lduw_be_p((void *)s); r = (v >> 7) & 0xf8; g = (v >> 2) & 0xf8; b = (v << 3) & 0xf8; @@ -305,15 +320,33 @@ static void vga_draw_line15(VGACommonState *s1, uint8_t *d, /* * 16 bit color */ -static void vga_draw_line16(VGACommonState *s1, uint8_t *d, -const uint8_t *s, int width) +static void vga_draw_line16_le(VGACommonState *s1, uint8_t *d, + const uint8_t *s, int width) { int w; uint32_t v, r, g, b; w = width; do { -v = lduw_p((void *)s); +v = lduw_le_p((void *)s); +r = (v >> 8) & 0xf8; +g = (v >> 3) & 0xfc; +b = (v << 3) & 0xf8; +((uint32_t *)d)[0] = rgb_to_pixel32(r, g, b); +s +=
[Qemu-devel] [PULL 10/10] vga: Add endian to vmstate
From: Benjamin Herrenschmidt Include the endian state in the migration stream as an optional subsection which we only include when the endian isn't the default, thus enabling backward compatibility of the common case. Signed-off-by: Benjamin Herrenschmidt Changes by kraxel: * Remove bochs dispi interface changes. We'll do that in a different way to make sure we don't conflict with possible future bochs dispi interface changes. * keep live migration bits. Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 41 + hw/display/vga_int.h | 2 ++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 49a4b8b..19e7f23 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1508,7 +1508,8 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) if (s->line_offset != s->last_line_offset || disp_width != s->last_width || height != s->last_height || -s->last_depth != depth) { +s->last_depth != depth || +s->last_byteswap != byteswap) { if (depth == 32 || (depth == 16 && !byteswap)) { pixman_format_code_t format = qemu_default_pixman_format(depth, !byteswap); @@ -1526,6 +1527,7 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) s->last_height = height; s->last_line_offset = s->line_offset; s->last_depth = depth; +s->last_byteswap = byteswap; full_update = 1; } else if (is_buffer_shared(surface) && (full_update || surface_data(surface) != s->vram_ptr @@ -1789,6 +1791,7 @@ void vga_common_reset(VGACommonState *s) s->cursor_start = 0; s->cursor_end = 0; s->cursor_offset = 0; +s->big_endian_fb = s->default_endian_fb; memset(s->invalidated_y_table, '\0', sizeof(s->invalidated_y_table)); memset(s->last_palette, '\0', sizeof(s->last_palette)); memset(s->last_ch_attr, '\0', sizeof(s->last_ch_attr)); @@ -2020,6 +2023,28 @@ static int vga_common_post_load(void *opaque, int version_id) return 0; } +static bool vga_endian_state_needed(void *opaque) +{ +VGACommonState *s = opaque; + +/* + * Only send the endian state if it's different from the + * default one, thus ensuring backward compatibility for + * migration of the common case + */ +return s->default_endian_fb != s->big_endian_fb; +} + +const VMStateDescription vmstate_vga_endian = { +.name = "vga.endian", +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_BOOL(big_endian_fb, VGACommonState), +VMSTATE_END_OF_LIST() +} +}; + const VMStateDescription vmstate_vga_common = { .name = "vga", .version_id = 2, @@ -2056,6 +2081,14 @@ const VMStateDescription vmstate_vga_common = { VMSTATE_UINT32(vbe_line_offset, VGACommonState), VMSTATE_UINT32(vbe_bank_mask, VGACommonState), VMSTATE_END_OF_LIST() +}, +.subsections = (VMStateSubsection []) { +{ +.vmsd = &vmstate_vga_endian, +.needed = vga_endian_state_needed, +}, { +/* empty */ +} } }; @@ -2126,14 +2159,14 @@ void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) } /* - * Set default fb endian based on target, should probably be turned + * Set default fb endian based on target, could probably be turned * into a device attribute set by the machine/platform to remove * all target endian dependencies from this file. */ #ifdef TARGET_WORDS_BIGENDIAN -s->big_endian_fb = true; +s->default_endian_fb = true; #else -s->big_endian_fb = false; +s->default_endian_fb = false; #endif vga_dirty_log_start(s); } diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 28d67cf..ed69e06 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -150,6 +150,7 @@ typedef struct VGACommonState { uint32_t last_width, last_height; /* in chars or pixels */ uint32_t last_scr_width, last_scr_height; /* in pixels */ uint32_t last_depth; /* in bits */ +bool last_byteswap; uint8_t cursor_start, cursor_end; bool cursor_visible_phase; int64_t cursor_blink_time; @@ -158,6 +159,7 @@ typedef struct VGACommonState { bool full_update_text; bool full_update_gfx; bool big_endian_fb; +bool default_endian_fb; /* hardware mouse cursor support */ uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; void (*cursor_invalidate)(struct VGACommonState *s); -- 1.8.3.1
[Qemu-devel] [PULL 09/10] vga: Make fb endian a common state variable
From: Benjamin Herrenschmidt And initialize it based on target endian Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 32 +++- hw/display/vga_int.h | 1 + 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 9f60f88..49a4b8b 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1461,16 +1461,11 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) int disp_width, multi_scan, multi_run; uint8_t *d; uint32_t v, addr1, addr; -vga_draw_line_func *vga_draw_line; -#if defined(TARGET_WORDS_BIGENDIAN) -static const bool big_endian_fb = true; -#else -static const bool big_endian_fb = false; -#endif -#if defined(HOST_WORDS_BIGENDIAN) -static const bool byteswap = !big_endian_fb; +vga_draw_line_func *vga_draw_line = NULL; +#ifdef HOST_WORDS_BIGENDIAN +bool byteswap = !s->big_endian_fb; #else -static const bool byteswap = big_endian_fb; +bool byteswap = s->big_endian_fb; #endif full_update |= update_basic_params(s); @@ -1573,19 +1568,19 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) bits = 8; break; case 15: -v = big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; bits = 16; break; case 16: -v = big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; bits = 16; break; case 24: -v = big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; bits = 24; break; case 32: -v = big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; +v = s->big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; bits = 32; break; } @@ -2129,6 +2124,17 @@ void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate) s->update_retrace_info = vga_precise_update_retrace_info; break; } + +/* + * Set default fb endian based on target, should probably be turned + * into a device attribute set by the machine/platform to remove + * all target endian dependencies from this file. + */ +#ifdef TARGET_WORDS_BIGENDIAN +s->big_endian_fb = true; +#else +s->big_endian_fb = false; +#endif vga_dirty_log_start(s); } diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index 9073370..28d67cf 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -157,6 +157,7 @@ typedef struct VGACommonState { const GraphicHwOps *hw_ops; bool full_update_text; bool full_update_gfx; +bool big_endian_fb; /* hardware mouse cursor support */ uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32]; void (*cursor_invalidate)(struct VGACommonState *s); -- 1.8.3.1
[Qemu-devel] [PULL 06/10] cirrus: Remove non-32bpp cursor drawing
From: Benjamin Herrenschmidt We only draw cursor on non-shared surfaces (so it seems...) and these are always 32bpp Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/cirrus_vga.c | 64 +--- hw/display/cirrus_vga_template.h | 102 --- 2 files changed, 36 insertions(+), 130 deletions(-) delete mode 100644 hw/display/cirrus_vga_template.h diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 6f8d149..8a5b76c 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2171,20 +2171,44 @@ static void cirrus_cursor_invalidate(VGACommonState *s1) } } -#define DEPTH 8 -#include "cirrus_vga_template.h" - -#define DEPTH 16 -#include "cirrus_vga_template.h" - -#define DEPTH 32 -#include "cirrus_vga_template.h" +static void vga_draw_cursor_line(uint8_t *d1, + const uint8_t *src1, + int poffset, int w, + unsigned int color0, + unsigned int color1, + unsigned int color_xor) +{ +const uint8_t *plane0, *plane1; +int x, b0, b1; +uint8_t *d; + +d = d1; +plane0 = src1; +plane1 = src1 + poffset; +for (x = 0; x < w; x++) { +b0 = (plane0[x >> 3] >> (7 - (x & 7))) & 1; +b1 = (plane1[x >> 3] >> (7 - (x & 7))) & 1; +switch (b0 | (b1 << 1)) { +case 0: +break; +case 1: +((uint32_t *)d)[0] ^= color_xor; +break; +case 2: +((uint32_t *)d)[0] = color0; +break; +case 3: +((uint32_t *)d)[0] = color1; +break; +} +d += 4; +} +} static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) { CirrusVGAState *s = container_of(s1, CirrusVGAState, vga); -DisplaySurface *surface = qemu_console_surface(s->vga.con); -int w, h, bpp, x1, x2, poffset; +int w, h, x1, x2, poffset; unsigned int color0, color1; const uint8_t *palette, *src; uint32_t content; @@ -2238,24 +2262,8 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y) color1 = rgb_to_pixel32(c6_to_8(palette[0xf * 3]), c6_to_8(palette[0xf * 3 + 1]), c6_to_8(palette[0xf * 3 + 2])); -bpp = surface_bytes_per_pixel(surface); -d1 += x1 * bpp; -switch (surface_bits_per_pixel(surface)) { -default: -break; -case 8: -vga_draw_cursor_line_8(d1, src, poffset, w, color0, color1, 0xff); -break; -case 15: -vga_draw_cursor_line_16(d1, src, poffset, w, color0, color1, 0x7fff); -break; -case 16: -vga_draw_cursor_line_16(d1, src, poffset, w, color0, color1, 0x); -break; -case 32: -vga_draw_cursor_line_32(d1, src, poffset, w, color0, color1, 0xff); -break; -} +d1 += x1 * 4; +vga_draw_cursor_line(d1, src, poffset, w, color0, color1, 0xff); } /*** diff --git a/hw/display/cirrus_vga_template.h b/hw/display/cirrus_vga_template.h deleted file mode 100644 index 3b28280..000 --- a/hw/display/cirrus_vga_template.h +++ /dev/null @@ -1,102 +0,0 @@ -/* - * QEMU Cirrus VGA Emulator templates - * - * Copyright (c) 2003 Fabrice Bellard - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -#if DEPTH == 8 -#define BPP 1 -#elif DEPTH == 15 || DEPTH == 16 -#define BPP 2 -#elif DEPTH == 32 -#define BPP 4 -#else -#error unsupported depth -#endif - -static void glue(vga_draw_cursor_line_, DEPTH)(uint8_t *d1, - const uint8_t *src1, - int poffset, int w, -
[Qemu-devel] [PULL 05/10] vga: Simplify vga_draw_blank() a bit
From: Benjamin Herrenschmidt The test for surface_bits_per_pixel() isn't necessary anymore, the 8bpp case never happens. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 9d06691..c3df0c5 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1682,7 +1682,7 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) static void vga_draw_blank(VGACommonState *s, int full_update) { DisplaySurface *surface = qemu_console_surface(s->con); -int i, w, val; +int i, w; uint8_t *d; if (!full_update) @@ -1690,15 +1690,10 @@ static void vga_draw_blank(VGACommonState *s, int full_update) if (s->last_scr_width <= 0 || s->last_scr_height <= 0) return; -if (surface_bits_per_pixel(surface) == 8) { -val = rgb_to_pixel32(0, 0, 0); -} else { -val = 0; -} w = s->last_scr_width * surface_bytes_per_pixel(surface); d = surface_data(surface); for(i = 0; i < s->last_scr_height; i++) { -memset(d, val, w); +memset(d, 0, w); d += surface_stride(surface); } dpy_gfx_update(s->con, 0, 0, -- 1.8.3.1
[Qemu-devel] [PULL 01/10] vga: Start cutting out non-32bpp conversion support
From: Benjamin Herrenschmidt Nowadays, we either share a surface with the host, or we create a 32bpp ARGB console surface. So we only need to draw/convert to 32bpp, enabling us to remove all but one instance of vga_template.h inclusion (to be further cleaned up), rgb_to_pixel_* etc... Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 258 +-- 1 file changed, 22 insertions(+), 236 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index df0c010..47e5d70 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1006,81 +1006,12 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) } } -typedef void vga_draw_glyph8_func(uint8_t *d, int linesize, - const uint8_t *font_ptr, int h, - uint32_t fgcol, uint32_t bgcol); -typedef void vga_draw_glyph9_func(uint8_t *d, int linesize, - const uint8_t *font_ptr, int h, - uint32_t fgcol, uint32_t bgcol, int dup9); typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, const uint8_t *s, int width); -#define DEPTH 8 -#include "vga_template.h" - -#define DEPTH 15 -#include "vga_template.h" - -#define BGR_FORMAT -#define DEPTH 15 -#include "vga_template.h" - -#define DEPTH 16 -#include "vga_template.h" - -#define BGR_FORMAT -#define DEPTH 16 -#include "vga_template.h" - #define DEPTH 32 #include "vga_template.h" -#define BGR_FORMAT -#define DEPTH 32 -#include "vga_template.h" - -static unsigned int rgb_to_pixel8_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel8(r, g, b); -col |= col << 8; -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel15_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel15(r, g, b); -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel15bgr_dup(unsigned int r, unsigned int g, - unsigned int b) -{ -unsigned int col; -col = rgb_to_pixel15bgr(r, g, b); -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel16_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel16(r, g, b); -col |= col << 16; -return col; -} - -static unsigned int rgb_to_pixel16bgr_dup(unsigned int r, unsigned int g, - unsigned int b) -{ -unsigned int col; -col = rgb_to_pixel16bgr(r, g, b); -col |= col << 16; -return col; -} static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) { @@ -1089,13 +1020,6 @@ static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned return col; } -static unsigned int rgb_to_pixel32bgr_dup(unsigned int r, unsigned int g, unsigned b) -{ -unsigned int col; -col = rgb_to_pixel32bgr(r, g, b); -return col; -} - /* return true if the palette was modified */ static int update_palette16(VGACommonState *s) { @@ -1202,56 +1126,6 @@ static int update_basic_params(VGACommonState *s) return full_update; } -#define NB_DEPTHS 7 - -static inline int get_depth_index(DisplaySurface *s) -{ -switch (surface_bits_per_pixel(s)) { -default: -case 8: -return 0; -case 15: -return 1; -case 16: -return 2; -case 32: -if (is_surface_bgr(s)) { -return 4; -} else { -return 3; -} -} -} - -static vga_draw_glyph8_func * const vga_draw_glyph8_table[NB_DEPTHS] = { -vga_draw_glyph8_8, -vga_draw_glyph8_16, -vga_draw_glyph8_16, -vga_draw_glyph8_32, -vga_draw_glyph8_32, -vga_draw_glyph8_16, -vga_draw_glyph8_16, -}; - -static vga_draw_glyph8_func * const vga_draw_glyph16_table[NB_DEPTHS] = { -vga_draw_glyph16_8, -vga_draw_glyph16_16, -vga_draw_glyph16_16, -vga_draw_glyph16_32, -vga_draw_glyph16_32, -vga_draw_glyph16_16, -vga_draw_glyph16_16, -}; - -static vga_draw_glyph9_func * const vga_draw_glyph9_table[NB_DEPTHS] = { -vga_draw_glyph9_8, -vga_draw_glyph9_16, -vga_draw_glyph9_16, -vga_draw_glyph9_32, -vga_draw_glyph9_32, -vga_draw_glyph9_16, -vga_draw_glyph9_16, -}; static const uint8_t cursor_glyph[32 * 4] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, @@ -1303,18 +1177,6 @@ static void vga_get_text_resolution(VGACommonState *s, int *pwidth, int *pheight *pcheight = cheight; } -typedef unsigned int rgb_to_pixel_dup_func(unsigned int r, unsigned int g, unsigned b); - -static rgb_to_pixel_dup_func * const rgb_to_pixel_dup_table[NB_DEPTHS] = { -rgb_to_pixel8_dup, -rgb_to_pixel15_dup, -rgb_to_pixel16_dup, -rgb_to_pixel32_dup, -rgb_to_pixel32bgr
[Qemu-devel] [PULL 07/10] vga: Remove some "should be done in BIOS" comments
From: Benjamin Herrenschmidt Not all platforms have a VGA BIOS, powerpc typically relies on using the DISPI interface to initialize the card. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index c3df0c5..21aa2c6 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -761,14 +761,13 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val) s->vbe_regs[VBE_DISPI_INDEX_ENABLE] |= VBE_DISPI_ENABLED; vbe_fixup_regs(s); -/* clear the screen (should be done in BIOS) */ +/* clear the screen */ if (!(val & VBE_DISPI_NOCLEARMEM)) { memset(s->vram_ptr, 0, s->vbe_regs[VBE_DISPI_INDEX_YRES] * s->vbe_line_offset); } -/* we initialize the VGA graphic mode (should be done - in BIOS) */ +/* we initialize the VGA graphic mode */ /* graphic mode + memory map 1 */ s->gr[VGA_GFX_MISC] = (s->gr[VGA_GFX_MISC] & ~0x0c) | 0x04 | VGA_GR06_GRAPHICS_MODE; @@ -801,7 +800,6 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val) (shift_control << 5); s->cr[VGA_CRTC_MAX_SCAN] &= ~0x9f; /* no double scan */ } else { -/* XXX: the bios should do that */ s->bank_offset = 0; } s->dac_8bit = (val & VBE_DISPI_8BIT_DAC) > 0; -- 1.8.3.1
[Qemu-devel] [PULL 02/10] vga: Remove remainder of old conversion cruft
From: Benjamin Herrenschmidt All the macros used to generate different versions of vga_template.h are now unnecessary, take them all out and remove the _32 suffix from most functions. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/vga.c | 46 +- hw/display/vga_template.h | 227 +++--- 2 files changed, 95 insertions(+), 178 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 47e5d70..721309f 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1009,10 +1009,8 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, const uint8_t *s, int width); -#define DEPTH 32 #include "vga_template.h" - static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) { unsigned int col; @@ -1311,19 +1309,19 @@ static void vga_draw_text(VGACommonState *s, int full_update) bgcol = palette[cattr >> 4]; fgcol = palette[cattr & 0x0f]; if (cw == 16) { -vga_draw_glyph16_32(d1, linesize, -font_ptr, cheight, fgcol, bgcol); +vga_draw_glyph16(d1, linesize, + font_ptr, cheight, fgcol, bgcol); } else if (cw != 9) { -vga_draw_glyph8_32(d1, linesize, - font_ptr, cheight, fgcol, bgcol); +vga_draw_glyph8(d1, linesize, +font_ptr, cheight, fgcol, bgcol); } else { dup9 = 0; if (ch >= 0xb0 && ch <= 0xdf && (s->ar[VGA_ATC_MODE] & 0x04)) { dup9 = 1; } -vga_draw_glyph9_32(d1, linesize, - font_ptr, cheight, fgcol, bgcol, dup9); +vga_draw_glyph9(d1, linesize, +font_ptr, cheight, fgcol, bgcol, dup9); } if (src == cursor_ptr && !(s->cr[VGA_CRTC_CURSOR_START] & 0x20) && @@ -1339,14 +1337,14 @@ static void vga_draw_text(VGACommonState *s, int full_update) h = line_last - line_start + 1; d = d1 + linesize * line_start; if (cw == 16) { -vga_draw_glyph16_32(d, linesize, - cursor_glyph, h, fgcol, bgcol); +vga_draw_glyph16(d, linesize, + cursor_glyph, h, fgcol, bgcol); } else if (cw != 9) { -vga_draw_glyph8_32(d, linesize, - cursor_glyph, h, fgcol, bgcol); +vga_draw_glyph8(d, linesize, +cursor_glyph, h, fgcol, bgcol); } else { -vga_draw_glyph9_32(d, linesize, - cursor_glyph, h, fgcol, bgcol, 1); +vga_draw_glyph9(d, linesize, +cursor_glyph, h, fgcol, bgcol, 1); } } } @@ -1384,16 +1382,16 @@ enum { }; static vga_draw_line_func * const vga_draw_line_table[VGA_DRAW_LINE_NB] = { -vga_draw_line2_32, -vga_draw_line2d2_32, -vga_draw_line4_32, -vga_draw_line4d2_32, -vga_draw_line8d2_32, -vga_draw_line8_32, -vga_draw_line15_32, -vga_draw_line16_32, -vga_draw_line24_32, -vga_draw_line32_32, +vga_draw_line2, +vga_draw_line2d2, +vga_draw_line4, +vga_draw_line4d2, +vga_draw_line8d2, +vga_draw_line8, +vga_draw_line15, +vga_draw_line16, +vga_draw_line24, +vga_draw_line32, }; static int vga_get_bpp(VGACommonState *s) diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h index 90ec9c2..0660b52 100644 --- a/hw/display/vga_template.h +++ b/hw/display/vga_template.h @@ -22,41 +22,9 @@ * THE SOFTWARE. */ -#if DEPTH == 8 -#define BPP 1 -#define PIXEL_TYPE uint8_t -#elif DEPTH == 15 || DEPTH == 16 -#define BPP 2 -#define PIXEL_TYPE uint16_t -#elif DEPTH == 32 -#define BPP 4 -#define PIXEL_TYPE uint32_t -#else -#error unsupport depth -#endif - -#ifdef BGR_FORMAT -#define PIXEL_NAME glue(DEPTH, bgr) -#else -#define PIXEL_NAME DEPTH -#endif /* BGR_FORMAT */ - -#if DEPTH != 15 && !defined(BGR_FORMAT) - -static inline void glue(vga_draw_glyph_line_, DEPTH)(uint8_t *d, - uint32_t font_data, - u
[Qemu-devel] [PULL 08/10] vga: Rename vga_template.h to vga-helpers.h
From: Benjamin Herrenschmidt It's no longer a template, we only instanciate the file once. Keep it a #included file so the functions remain static. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Gerd Hoffmann Reviewed-by: David Gibson --- hw/display/{vga_template.h => vga-helpers.h} | 0 hw/display/vga.c | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename hw/display/{vga_template.h => vga-helpers.h} (100%) diff --git a/hw/display/vga_template.h b/hw/display/vga-helpers.h similarity index 100% rename from hw/display/vga_template.h rename to hw/display/vga-helpers.h diff --git a/hw/display/vga.c b/hw/display/vga.c index 21aa2c6..9f60f88 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1007,7 +1007,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d, const uint8_t *s, int width); -#include "vga_template.h" +#include "vga-helpers.h" /* return true if the palette was modified */ static int update_palette16(VGACommonState *s) -- 1.8.3.1
[Qemu-devel] [PULL 0/1] pixman: fix qemu_default_pixman_format (32bpp non-native endian)
Hi, Fix pixman format bug. please pull, Gerd The following changes since commit 29429c7244c73eefada3d0ec6dd30c5698782d08: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140929' into staging (2014-09-30 11:02:06 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-console-20141002-1 for you to fetch changes up to 89ec031b09c81821e349c364575fad652395cf94: pixman: fix qemu_default_pixman_format (32bpp non-native endian) (2014-09-30 13:34:04 +0200) pixman: fix qemu_default_pixman_format (32bpp non-native endian) Gerd Hoffmann (1): pixman: fix qemu_default_pixman_format (32bpp non-native endian) ui/qemu-pixman.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PULL 1/1] pixman: fix qemu_default_pixman_format (32bpp non-native endian)
Bug breaks SDL display of bigendian guests on little endian hosts. Reported-by: BALATON Zoltan Reported-by: Valentin Manea Signed-off-by: Gerd Hoffmann --- ui/qemu-pixman.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c index 30c7fdd..1f6fea5 100644 --- a/ui/qemu-pixman.c +++ b/ui/qemu-pixman.c @@ -80,7 +80,7 @@ pixman_format_code_t qemu_default_pixman_format(int bpp, bool native_endian) case 24: return PIXMAN_b8g8r8; case 32: -return PIXMAN_b8g8r8a8; +return PIXMAN_b8g8r8x8; break; } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH 1/3] pc: Fix disabling of vapic for compat PC models
On Thu, Oct 02, 2014 at 09:27:41AM +0200, Jan Kiszka wrote: > On 2014-09-02 17:11, Michael Roth wrote: > > Quoting Markus Armbruster (2014-07-30 06:19:36) > >> Paolo Bonzini writes: > >> > >>> Il 30/07/2014 10:57, Michael S. Tsirkin ha scritto: > On Wed, Jul 30, 2014 at 09:01:59AM +0200, Jan Kiszka wrote: > > We used to be able to address both the QEMU and the KVM APIC via "apic". > > This doesn't work anymore. So we need to use their parent class to turn > > off the vapic on machines that should not expose them. > > > > Signed-off-by: Jan Kiszka > > > > OK so this is intended for 2.2? > >> > >> If yes, we should cc: qemu-stable. > > > > Ping for stable 2.1.1, freeze is on Wednesday > > Lost track of this: was I supposed to provide anything different, or did > this just fall under the table? > > Jan Yes, I think Michael expected an ACK for stable. Oh well. Would you like me to apply as is, or to rework this to avoid duplication of string names? > > > >> > In that case, how about creating a macro with type name, > and using that? This way things don't break if we rename > something again. > >>> > >>> Don't we have warnings for that now? > >> > >> Warnings don't help much in cases like this: "apic" still exists and has > >> the property, it's just not the device we want. Macros aren't > >> foolproof, either. > >> > > --- > > hw/i386/pc_piix.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > index 9694f88..73ba77d 100644 > > --- a/hw/i386/pc_piix.c > > +++ b/hw/i386/pc_piix.c > > @@ -645,7 +645,7 @@ static QEMUMachine pc_machine_v1_1 = { > > .property = "class",\ > > .value= stringify(PCI_CLASS_MEMORY_RAM),\ > > },{\ > > -.driver = "apic",\ > > +.driver = "apic-common",\ > > .property = "vapic",\ > > .value= "off",\ > > },{\ > > -- > > 1.8.1.1.298.ge7eed54 > >> > >> You could use TYPE_APIC_COMMON here. Including > >> "hw/i386/apic_internal.h" for it would be not so nice, though. > > > > -- > Siemens AG, Corporate Technology, CT RTC ITP SES-DE > Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] configure: Prepend pixman and ftd flags to overrule system-provided ones
Other packages may provide includes for pixman as well if the host has a devel package installed. So add ours to the front to unsure that the right version is used. Signed-off-by: Jan Kiszka --- Replaces "configure: Filter out system includes for pixman" And, yes, the scenario is too old distro pixman. configure | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 9ac2600..a9e4d49 100755 --- a/configure +++ b/configure @@ -4211,9 +4211,9 @@ EOF fi fi -# add pixman flags after all config tests are done -QEMU_CFLAGS="$QEMU_CFLAGS $pixman_cflags $fdt_cflags" -libs_softmmu="$libs_softmmu $pixman_libs" +# prepend pixman and ftd flags after all config tests are done +QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS" +libs_softmmu="$pixman_libs $libs_softmmu" echo "Install prefix$prefix" echo "BIOS directory`eval echo $qemu_datadir`" -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [PATCH 1/3] pc: Fix disabling of vapic for compat PC models
On 2014-10-02 10:03, Michael S. Tsirkin wrote: > On Thu, Oct 02, 2014 at 09:27:41AM +0200, Jan Kiszka wrote: >> On 2014-09-02 17:11, Michael Roth wrote: >>> Quoting Markus Armbruster (2014-07-30 06:19:36) Paolo Bonzini writes: > Il 30/07/2014 10:57, Michael S. Tsirkin ha scritto: >> On Wed, Jul 30, 2014 at 09:01:59AM +0200, Jan Kiszka wrote: >>> We used to be able to address both the QEMU and the KVM APIC via "apic". >>> This doesn't work anymore. So we need to use their parent class to turn >>> off the vapic on machines that should not expose them. >>> >>> Signed-off-by: Jan Kiszka >> >> >> >> OK so this is intended for 2.2? If yes, we should cc: qemu-stable. >>> >>> Ping for stable 2.1.1, freeze is on Wednesday >> >> Lost track of this: was I supposed to provide anything different, or did >> this just fall under the table? >> >> Jan > > Yes, I think Michael expected an ACK for stable. > Oh well. > Would you like me to apply as is, or to rework this to avoid duplication > of string names? I don't mind, but I wouldn't refuse if you want to take care of the issue. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PULL 2/2] add input-send-event command
From: Marcelo Tosatti Which allows specification of absolute/relative, up/down and console parameters. Suggested by Gerd Hoffman. Signed-off-by: Marcelo Tosatti Reviewed-by: Eric Blake Signed-off-by: Gerd Hoffmann --- qapi-schema.json | 17 +++ qmp-commands.hx | 63 ui/input.c | 37 + 3 files changed, 117 insertions(+) diff --git a/qapi-schema.json b/qapi-schema.json index 4bfaf20..2e9e261 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3233,6 +3233,23 @@ 'abs' : 'InputMoveEvent' } } ## +# @input-send-event +# +# Send input event(s) to guest. +# +# @console: Which console to send event(s) to. +# +# @events: List of InputEvent union. +# +# Returns: Nothing on success. +# +# Since: 2.2 +# +## +{ 'command': 'input-send-event', + 'data': { 'console':'int', 'events': [ 'InputEvent' ] } } + +## # @NumaOptions # # A discriminated record of NUMA options. (for OptsVisitor) diff --git a/qmp-commands.hx b/qmp-commands.hx index f581813..1abd619 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3789,3 +3789,66 @@ Example: -> { "execute": "trace-event-set-state", "arguments": { "name": "qemu_memalign", "enable": "true" } } <- { "return": {} } EQMP + +{ +.name = "input-send-event", +.args_type = "console:i,events:q", +.mhandler.cmd_new = qmp_marshal_input_input_send_event, +}, + +SQMP +@input-send-event +- + +Send input event to guest. + +Arguments: + +- "console": console index. +- "events": list of input events. + +The consoles are visible in the qom tree, under +/backend/console[$index]. They have a device link and head property, so +it is possible to map which console belongs to which device and display. + +Example (1): + +Press left mouse button. + +-> { "execute": "input-send-event", +"arguments": { "console": 0, + "events": [ { "type": "btn", +"data" : { "down": true, "button": "Left" } } } } +<- { "return": {} } + +-> { "execute": "input-send-event", +"arguments": { "console": 0, + "events": [ { "type": "btn", +"data" : { "down": false, "button": "Left" } } } } +<- { "return": {} } + +Example (2): + +Press ctrl-alt-del. + +-> { "execute": "input-send-event", + "arguments": { "console": 0, "events": [ +{ "type": "key", "data" : { "down": true, + "key": {"type": "qcode", "data": "ctrl" } } }, +{ "type": "key", "data" : { "down": true, + "key": {"type": "qcode", "data": "alt" } } }, +{ "type": "key", "data" : { "down": true, + "key": {"type": "qcode", "data": "delete" } } } ] } } +<- { "return": {} } + +Example (3): + +Move mouse pointer to absolute coordinates (2, 400). + +-> { "execute": "input-send-event" , + "arguments": { "console": 0, "events": [ + { "type": "abs", "data" : { "axis": "X", "value" : 2 } }, + { "type": "abs", "data" : { "axis": "Y", "value" : 400 } } ] } } +<- { "return": {} } + +EQMP diff --git a/ui/input.c b/ui/input.c index 89d9db7..002831e 100644 --- a/ui/input.c +++ b/ui/input.c @@ -122,6 +122,43 @@ qemu_input_find_handler(uint32_t mask, QemuConsole *con) return NULL; } +void qmp_input_send_event(int64_t console, InputEventList *events, + Error **errp) +{ +InputEventList *e; +QemuConsole *con; + +con = qemu_console_lookup_by_index(console); +if (!con) { +error_setg(errp, "console %" PRId64 " not found", console); +return; +} + +if (!runstate_is_running() && !runstate_check(RUN_STATE_SUSPENDED)) { +error_setg(errp, "VM not running"); +return; +} + +for (e = events; e != NULL; e = e->next) { +InputEvent *event = e->value; + +if (!qemu_input_find_handler(1 << event->kind, con)) { +error_setg(errp, "Input handler not found for " + "event type %s", +InputEventKind_lookup[event->kind]); +return; +} +} + +for (e = events; e != NULL; e = e->next) { +InputEvent *event = e->value; + +qemu_input_event_send(con, event); +} + +qemu_input_event_sync(); +} + static void qemu_input_transform_abs_rotate(InputEvent *evt) { switch (graphic_rotate) { -- 1.8.3.1
[Qemu-devel] [PULL 0/2] input patch queue
Hi, Two input monitor patches: Fix fix send-key release ordering & add new input-send-event qmp command. please pull, Gerd The following changes since commit 29429c7244c73eefada3d0ec6dd30c5698782d08: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140929' into staging (2014-09-30 11:02:06 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-input-20141002-1 for you to fetch changes up to 50c6617fcbef649674b59f686b12dccfc455ffa3: add input-send-event command (2014-10-02 09:58:14 +0200) input monitor patches: fix send-key release ordering and new input-send-event command Gerd Hoffmann (1): input: fix send-key monitor command release event ordering Marcelo Tosatti (1): add input-send-event command qapi-schema.json | 17 +++ qmp-commands.hx | 63 +++ ui/input-legacy.c | 11 -- ui/input.c| 37 4 files changed, 126 insertions(+), 2 deletions(-)
[Qemu-devel] [PULL 1/2] input: fix send-key monitor command release event ordering
commit 2e377f1730d06deafb3e3ef6cf88792de4a6f4df changed the ordering of the release events as side effect. Some guests are not happy with that and don't recognise ctrl-alt-del any more. This patch restores the old last-pressed first-released behavior. Cc: Amos Kong Signed-off-by: Gerd Hoffmann --- ui/input-legacy.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ui/input-legacy.c b/ui/input-legacy.c index 3025f50..a698a34 100644 --- a/ui/input-legacy.c +++ b/ui/input-legacy.c @@ -85,6 +85,8 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time, Error **errp) { KeyValueList *p; +KeyValue **up = NULL; +int count = 0; if (!has_hold_time) { hold_time = 0; /* use default */ @@ -93,11 +95,16 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time, for (p = keys; p != NULL; p = p->next) { qemu_input_event_send_key(NULL, copy_key_value(p->value), true); qemu_input_event_send_key_delay(hold_time); +up = g_realloc(up, sizeof(*up) * (count+1)); +up[count] = copy_key_value(p->value); +count++; } -for (p = keys; p != NULL; p = p->next) { -qemu_input_event_send_key(NULL, copy_key_value(p->value), false); +while (count) { +count--; +qemu_input_event_send_key(NULL, up[count], false); qemu_input_event_send_key_delay(hold_time); } +g_free(up); } static void legacy_kbd_event(DeviceState *dev, QemuConsole *src, -- 1.8.3.1
Re: [Qemu-devel] [PATCH 1/3] pc: Fix disabling of vapic for compat PC models
On Thu, Oct 02, 2014 at 10:05:38AM +0200, Jan Kiszka wrote: > On 2014-10-02 10:03, Michael S. Tsirkin wrote: > > On Thu, Oct 02, 2014 at 09:27:41AM +0200, Jan Kiszka wrote: > >> On 2014-09-02 17:11, Michael Roth wrote: > >>> Quoting Markus Armbruster (2014-07-30 06:19:36) > Paolo Bonzini writes: > > > Il 30/07/2014 10:57, Michael S. Tsirkin ha scritto: > >> On Wed, Jul 30, 2014 at 09:01:59AM +0200, Jan Kiszka wrote: > >>> We used to be able to address both the QEMU and the KVM APIC via > >>> "apic". > >>> This doesn't work anymore. So we need to use their parent class to > >>> turn > >>> off the vapic on machines that should not expose them. > >>> > >>> Signed-off-by: Jan Kiszka > >> > >> > >> > >> OK so this is intended for 2.2? > > If yes, we should cc: qemu-stable. > >>> > >>> Ping for stable 2.1.1, freeze is on Wednesday > >> > >> Lost track of this: was I supposed to provide anything different, or did > >> this just fall under the table? > >> > >> Jan > > > > Yes, I think Michael expected an ACK for stable. > > Oh well. > > Would you like me to apply as is, or to rework this to avoid duplication > > of string names? > > I don't mind, but I wouldn't refuse if you want to take care of the issue. > > Jan I've applied as-is for now, thanks. I think the use of strings in compat machine types is too fragile, we should have a header with type names, and reuse it through macros, but apic is not unique here. > -- > Siemens AG, Corporate Technology, CT RTC ITP SES-DE > Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 0/2 V5] Virtual Machine Generation ID
On 01/10/2014 11:58, Markus Armbruster wrote: Did this get stuck? I think so. Did I miss a comment which was not handled in the last version of the patch? Gal. Gal Hammer writes: Hi, A two parts patch to add a QEmu support for Microsoft's Virtual Machine Generation ID device. The first one add a new ACPI directive which allow to use a 16-bytes buffer in an ACPI table. This buffer is for storing the VM's UUID. The second is the ACPI tables changes and the actual device. Your comment are welcomed. Thanks, Gal. V5: - include the pre-compiled ASL file - remove an empty line at end of files. V4: - Move device's description to SSDT table (dynamic build). V3: - Fix a typo in error message string. - Move device's description from DSDT back to SSDT table. V2: - Remove "-uuid" command line parameter. - Move device's description from SSDT to DSDT table. - Add new "vmgenid" sysbus device.
Re: [Qemu-devel] [PATCH 0/2 V5] Virtual Machine Generation ID
On Thu, Oct 02, 2014 at 11:32:42AM +0300, Gal Hammer wrote: > On 01/10/2014 11:58, Markus Armbruster wrote: > >Did this get stuck? > > I think so. Did I miss a comment which was not handled in the last version > of the patch? > > Gal. I don't think you did but you have to Cc maintainers if you want your patches to be applied :) I'll queue this for review, thanks! > >Gal Hammer writes: > > > >>Hi, > >> > >>A two parts patch to add a QEmu support for Microsoft's Virtual Machine > >>Generation ID device. > >> > >>The first one add a new ACPI directive which allow to use a 16-bytes > >>buffer in an ACPI table. This buffer is for storing the VM's UUID. > >> > >>The second is the ACPI tables changes and the actual device. > >> > >>Your comment are welcomed. > >> > >>Thanks, > >> > >> Gal. > >> > >>V5: - include the pre-compiled ASL file > >> - remove an empty line at end of files. > >> > >>V4: - Move device's description to SSDT table (dynamic build). > >> > >>V3: - Fix a typo in error message string. > >> - Move device's description from DSDT back to SSDT table. > >> > >>V2: - Remove "-uuid" command line parameter. > >> - Move device's description from SSDT to DSDT table. > >> - Add new "vmgenid" sysbus device. >
Re: [Qemu-devel] pending target-mips patches
Hi Peter, On 01/10/2014 17:32, Peter Maydell wrote: > On 1 October 2014 16:35, Leon Alrae wrote: >> I noticed that it's quite difficult to get target-mips changes >> reviewed/accepted. There is already a queue of relatively big features >> and bug fixes which are stuck for months. Does anyone have an idea how >> to improve this situation? Wouldn't it help to have a target-mips >> co-maintainer assisting Aurelien? > > I agree that an active co-maintainer for target-mips would > be a good idea. Is anybody volunteering? (Are you? :-)) I would be happy to help maintaining target-mips, but I'm wondering if there are any more experienced (especially in legacy MIPS) volunteers. > If somebody wants to take on this job, a good first start > would be to assemble a git tree of patches which have got > code review (ie which have accumulated at least one > reviewed-by tag and aren't the subject of on-list > disagreement about whether they're correct), test it, and > submit it as a pull request (in the right format, and with > your signed-off-by lines on the patches and any reviewed-by > or acked-by lines from the list)... If somebody does that > I will take a look at the result and if I'm happy with it > I'll apply them. Sounds straightforward, thanks. Regards, Leon
[Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate for incoming migration
When migrated using libvirt with "--copy-storage-all", at the end of migration there is race between NBD mirroring task trying to do flush and migration completion, both end up invalidating cache. Since qcow2 driver does not handle this situation very well, random crashes happen. This disables the BDRV_O_INCOMING flag for the block device being migrated and restores it when NBD task is done. Signed-off-by: Alexey Kardashevskiy --- The commit log is not full and most likely incorrect as well as the patch :) Please, help. Thanks! The patch seems to fix the initial problem though. btw is there any easy way to migrate one QEMU to another using NBD (i.e. not using "migrate -b") and not using libvirt? What would the command line be? Debugging with libvirt is real pain :( --- block.c | 17 - migration.c | 1 - nbd.c | 11 +++ 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/block.c b/block.c index c5a251c..ed72e0a 100644 --- a/block.c +++ b/block.c @@ -5073,6 +5073,10 @@ void bdrv_invalidate_cache_all(Error **errp) QTAILQ_FOREACH(bs, &bdrv_states, device_list) { AioContext *aio_context = bdrv_get_aio_context(bs); +if (!(bs->open_flags & BDRV_O_INCOMING)) { +continue; +} + aio_context_acquire(aio_context); bdrv_invalidate_cache(bs, &local_err); aio_context_release(aio_context); @@ -5083,19 +5087,6 @@ void bdrv_invalidate_cache_all(Error **errp) } } -void bdrv_clear_incoming_migration_all(void) -{ -BlockDriverState *bs; - -QTAILQ_FOREACH(bs, &bdrv_states, device_list) { -AioContext *aio_context = bdrv_get_aio_context(bs); - -aio_context_acquire(aio_context); -bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING); -aio_context_release(aio_context); -} -} - int bdrv_flush(BlockDriverState *bs) { Coroutine *co; diff --git a/migration.c b/migration.c index 8d675b3..c49a05a 100644 --- a/migration.c +++ b/migration.c @@ -103,7 +103,6 @@ static void process_incoming_migration_co(void *opaque) } qemu_announce_self(); -bdrv_clear_incoming_migration_all(); /* Make sure all file formats flush their mutable metadata */ bdrv_invalidate_cache_all(&local_err); if (local_err) { diff --git a/nbd.c b/nbd.c index e9b539b..7b479c0 100644 --- a/nbd.c +++ b/nbd.c @@ -106,6 +106,7 @@ struct NBDExport { off_t dev_offset; off_t size; uint32_t nbdflags; +bool restore_incoming; QTAILQ_HEAD(, NBDClient) clients; QTAILQ_ENTRY(NBDExport) next; @@ -972,6 +973,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, exp->ctx = bdrv_get_aio_context(bs); bdrv_ref(bs); bdrv_add_aio_context_notifier(bs, bs_aio_attached, bs_aio_detach, exp); + +if (bs->open_flags & BDRV_O_INCOMING) { +bdrv_invalidate_cache(bs, NULL); +exp->restore_incoming = !!(bs->open_flags & BDRV_O_INCOMING); +bs->open_flags &= ~(BDRV_O_INCOMING); +} + return exp; } @@ -1021,6 +1029,9 @@ void nbd_export_close(NBDExport *exp) if (exp->bs) { bdrv_remove_aio_context_notifier(exp->bs, bs_aio_attached, bs_aio_detach, exp); +if (exp->restore_incoming) { +exp->bs->open_flags |= BDRV_O_INCOMING; +} bdrv_unref(exp->bs); exp->bs = NULL; } -- 2.0.0
[Qemu-devel] [PATCH v5 01/23] block: Split bdrv_new_root() off bdrv_new()
Creating an anonymous BDS can't fail. Make that obvious. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Benoît Canet Reviewed-by: Kevin Wolf --- block.c | 28 +++- block/iscsi.c | 2 +- block/vvfat.c | 2 +- blockdev.c| 2 +- hw/block/xen_disk.c | 2 +- include/block/block.h | 3 ++- qemu-img.c| 6 +++--- qemu-io.c | 2 +- qemu-nbd.c| 2 +- 9 files changed, 30 insertions(+), 19 deletions(-) diff --git a/block.c b/block.c index d3aebeb..3905c32 100644 --- a/block.c +++ b/block.c @@ -336,10 +336,11 @@ void bdrv_register(BlockDriver *bdrv) } /* create a new block device (by default it is empty) */ -BlockDriverState *bdrv_new(const char *device_name, Error **errp) +BlockDriverState *bdrv_new_root(const char *device_name, Error **errp) { BlockDriverState *bs; -int i; + +assert(*device_name); if (*device_name && !id_wellformed(device_name)) { error_setg(errp, "Invalid device name"); @@ -358,12 +359,21 @@ BlockDriverState *bdrv_new(const char *device_name, Error **errp) return NULL; } +bs = bdrv_new(); + +pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); +QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); + +return bs; +} + +BlockDriverState *bdrv_new(void) +{ +BlockDriverState *bs; +int i; + bs = g_new0(BlockDriverState, 1); QLIST_INIT(&bs->dirty_bitmaps); -pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); -if (device_name[0] != '\0') { -QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); -} for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { QLIST_INIT(&bs->op_blockers[i]); } @@ -1224,7 +1234,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) goto free_exit; } -backing_hd = bdrv_new("", errp); +backing_hd = bdrv_new(); if (bs->backing_format[0] != '\0') { back_drv = bdrv_find_format(bs->backing_format); @@ -1353,7 +1363,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) qdict_put(snapshot_options, "file.filename", qstring_from_str(tmp_filename)); -bs_snapshot = bdrv_new("", &error_abort); +bs_snapshot = bdrv_new(); ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options, flags, bdrv_qcow2, &local_err); @@ -1424,7 +1434,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, if (*pbs) { bs = *pbs; } else { -bs = bdrv_new("", &error_abort); +bs = bdrv_new(); } /* NULL means an empty set of options */ diff --git a/block/iscsi.c b/block/iscsi.c index 3a01de0..a7fb764 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1519,7 +1519,7 @@ static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp) IscsiLun *iscsilun = NULL; QDict *bs_options; -bs = bdrv_new("", &error_abort); +bs = bdrv_new(); /* Read out options */ total_size = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), diff --git a/block/vvfat.c b/block/vvfat.c index 731e591..6c9fde0 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2939,7 +2939,7 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp) unlink(s->qcow_filename); #endif -bdrv_set_backing_hd(s->bs, bdrv_new("", &error_abort)); +bdrv_set_backing_hd(s->bs, bdrv_new()); s->bs->backing_hd->drv = &vvfat_write_target; s->bs->backing_hd->opaque = g_new(void *, 1); *(void**)s->bs->backing_hd->opaque = s; diff --git a/blockdev.c b/blockdev.c index 2f441c5..cf06d6d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -461,7 +461,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, } /* init */ -bs = bdrv_new(qemu_opts_id(opts), errp); +bs = bdrv_new_root(qemu_opts_id(opts), errp); if (!bs) { goto early_err; } diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 0d27ab1..7f0f66b 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -858,7 +858,7 @@ static int blk_connect(struct XenDevice *xendev) /* setup via xenbus -> create new block driver instance */ xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n"); -blkdev->bs = bdrv_new(blkdev->dev, NULL); +blkdev->bs = bdrv_new_root(blkdev->dev, NULL); if (!blkdev->bs) { return -1; } diff --git a/include/block/block.h b/include/block/block.h index 3318f0d..a3039ce 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -204,7 +204,8 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name, int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts, Error **errp); int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp); -BlockDriverState *bdrv_new(c
[Qemu-devel] [PATCH v5 07/23] block: Eliminate bdrv_iterate(), use bdrv_next()
Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- block-migration.c | 30 +++--- block.c | 9 - blockdev.c| 31 +-- include/block/block.h | 2 -- monitor.c | 32 +--- 5 files changed, 37 insertions(+), 67 deletions(-) diff --git a/block-migration.c b/block-migration.c index 3ad31a2..cb3e16c 100644 --- a/block-migration.c +++ b/block-migration.c @@ -343,12 +343,25 @@ static void unset_dirty_tracking(void) } } -static void init_blk_migration_it(void *opaque, BlockDriverState *bs) +static void init_blk_migration(QEMUFile *f) { +BlockDriverState *bs; BlkMigDevState *bmds; int64_t sectors; -if (!bdrv_is_read_only(bs)) { +block_mig_state.submitted = 0; +block_mig_state.read_done = 0; +block_mig_state.transferred = 0; +block_mig_state.total_sector_sum = 0; +block_mig_state.prev_progress = -1; +block_mig_state.bulk_completed = 0; +block_mig_state.zero_blocks = migrate_zero_blocks(); + +for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { +if (bdrv_is_read_only(bs)) { +continue; +} + sectors = bdrv_nb_sectors(bs); if (sectors <= 0) { return; @@ -378,19 +391,6 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs) } } -static void init_blk_migration(QEMUFile *f) -{ -block_mig_state.submitted = 0; -block_mig_state.read_done = 0; -block_mig_state.transferred = 0; -block_mig_state.total_sector_sum = 0; -block_mig_state.prev_progress = -1; -block_mig_state.bulk_completed = 0; -block_mig_state.zero_blocks = migrate_zero_blocks(); - -bdrv_iterate(init_blk_migration_it, NULL); -} - /* Called with no lock taken. */ static int blk_mig_save_bulked_block(QEMUFile *f) diff --git a/block.c b/block.c index d6ab0c7..5fd173a 100644 --- a/block.c +++ b/block.c @@ -3906,15 +3906,6 @@ BlockDriverState *bdrv_next(BlockDriverState *bs) return QTAILQ_NEXT(bs, device_list); } -void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque) -{ -BlockDriverState *bs; - -QTAILQ_FOREACH(bs, &bdrv_states, device_list) { -it(opaque, bs); -} -} - const char *bdrv_get_device_name(BlockDriverState *bs) { return bs->device_name; diff --git a/blockdev.c b/blockdev.c index 07321b1..09ce9fa 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2550,26 +2550,21 @@ fail: qmp_output_visitor_cleanup(ov); } -static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs) -{ -BlockJobInfoList **prev = opaque; -BlockJob *job = bs->job; - -if (job) { -BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1); -elem->value = block_job_query(bs->job); -(*prev)->next = elem; -*prev = elem; -} -} - BlockJobInfoList *qmp_query_block_jobs(Error **errp) { -/* Dummy is a fake list element for holding the head pointer */ -BlockJobInfoList dummy = {}; -BlockJobInfoList *prev = &dummy; -bdrv_iterate(do_qmp_query_block_jobs_one, &prev); -return dummy.next; +BlockJobInfoList *head = NULL, **p_next = &head; +BlockDriverState *bs; + +for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) { +if (bs->job) { +BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1); +elem->value = block_job_query(bs->job); +*p_next = elem; +p_next = &elem->next; +} +} + +return head; } QemuOptsList qemu_common_drive_opts = { diff --git a/include/block/block.h b/include/block/block.h index a3039ce..6c97dd4 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -411,8 +411,6 @@ BlockDriverState *bdrv_lookup_bs(const char *device, Error **errp); bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base); BlockDriverState *bdrv_next(BlockDriverState *bs); -void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), - void *opaque); int bdrv_is_encrypted(BlockDriverState *bs); int bdrv_key_required(BlockDriverState *bs); int bdrv_set_key(BlockDriverState *bs, const char *key); diff --git a/monitor.c b/monitor.c index 2d14f39..412e63f 100644 --- a/monitor.c +++ b/monitor.c @@ -4216,24 +4216,6 @@ static void file_completion(Monitor *mon, const char *input) closedir(ffs); } -typedef struct MonitorBlockComplete { -Monitor *mon; -const char *input; -} MonitorBlockComplete; - -static void block_completion_it(void *opaque, BlockDriverState *bs) -{ -const char *name = bdrv_get_device_name(bs); -MonitorBlockComplete *mbc = opaque; -Monitor *mon = mbc->mon; -const char *input = mbc->input; - -if (input[0] == '\0' || -!strncmp(name, (char *)input, strlen(input))) { -readline_add_completion(mon->
[Qemu-devel] [PATCH v5 05/23] block: Code motion to get rid of stubs/blockdev.c
Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- block/block-backend.c | 13 + blockdev.c| 11 --- include/sysemu/blockdev.h | 1 - stubs/Makefile.objs | 1 - stubs/blockdev.c | 12 5 files changed, 13 insertions(+), 25 deletions(-) delete mode 100644 stubs/blockdev.c diff --git a/block/block-backend.c b/block/block-backend.c index edaf73c..d4bdd48 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -22,6 +22,8 @@ struct BlockBackend { QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */ }; +static void drive_info_del(DriveInfo *dinfo); + /* All the BlockBackends (except for hidden ones) */ static QTAILQ_HEAD(, BlockBackend) blk_backends = QTAILQ_HEAD_INITIALIZER(blk_backends); @@ -93,6 +95,17 @@ static void blk_delete(BlockBackend *blk) g_free(blk); } +static void drive_info_del(DriveInfo *dinfo) +{ +if (!dinfo) { +return; +} +qemu_opts_del(dinfo->opts); +g_free(dinfo->id); +g_free(dinfo->serial); +g_free(dinfo); +} + /* * Increment @blk's reference count. * @blk must not be null. diff --git a/blockdev.c b/blockdev.c index 3a6bbe9..7fcd26c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -223,17 +223,6 @@ void drive_del(DriveInfo *dinfo) blk_unref(blk_by_legacy_dinfo(dinfo)); } -void drive_info_del(DriveInfo *dinfo) -{ -if (!dinfo) { -return; -} -qemu_opts_del(dinfo->opts); -g_free(dinfo->id); -g_free(dinfo->serial); -g_free(dinfo); -} - typedef struct { QEMUBH *bh; BlockDriverState *bs; diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index 1dc5906..2ed297b 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -60,7 +60,6 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, const char *optstr); DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type); void drive_del(DriveInfo *dinfo); -void drive_info_del(DriveInfo *dinfo); /* device-hotplug */ diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index c0b1f6a..5e347d0 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -1,6 +1,5 @@ stub-obj-y += arch-query-cpu-def.o stub-obj-y += bdrv-commit-all.o -stub-obj-y += blockdev.o stub-obj-y += chr-baum-init.o stub-obj-y += chr-msmouse.o stub-obj-y += chr-testdev.o diff --git a/stubs/blockdev.c b/stubs/blockdev.c deleted file mode 100644 index 5d0a79c..000 --- a/stubs/blockdev.c +++ /dev/null @@ -1,12 +0,0 @@ -#include -#include "sysemu/blockdev.h" - -DriveInfo *drive_get_by_blockdev(BlockDriverState *bs) -{ -return NULL; -} - -void drive_info_del(DriveInfo *dinfo) -{ -assert(!dinfo); -} -- 1.9.3
[Qemu-devel] [PATCH v5 14/23] virtio-blk: Rename VirtIOBlkConf variables to conf
This is consistent with how VirtIOFOOConf variables are named elsewhere, and makes blk available for BlockBackend variables. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- hw/block/dataplane/virtio-blk.c | 33 ++- hw/block/dataplane/virtio-blk.h | 2 +- hw/block/virtio-blk.c | 50 - include/hw/virtio/virtio-blk.h | 2 +- 4 files changed, 44 insertions(+), 43 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 5458f9d..bd250df 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -30,7 +30,7 @@ struct VirtIOBlockDataPlane { bool stopping; bool disabled; -VirtIOBlkConf *blk; +VirtIOBlkConf *conf; VirtIODevice *vdev; Vring vring;/* virtqueue vring */ @@ -94,7 +94,7 @@ static void handle_notify(EventNotifier *e) VirtIOBlock *vblk = VIRTIO_BLK(s->vdev); event_notifier_test_and_clear(&s->host_notifier); -bdrv_io_plug(s->blk->conf.bs); +bdrv_io_plug(s->conf->conf.bs); for (;;) { MultiReqBuffer mrb = { .num_writes = 0, @@ -120,7 +120,7 @@ static void handle_notify(EventNotifier *e) virtio_blk_handle_request(req, &mrb); } -virtio_submit_multiwrite(s->blk->conf.bs, &mrb); +virtio_submit_multiwrite(s->conf->conf.bs, &mrb); if (likely(ret == -EAGAIN)) { /* vring emptied */ /* Re-enable guest->host notifies and stop processing the vring. @@ -133,11 +133,11 @@ static void handle_notify(EventNotifier *e) break; } } -bdrv_io_unplug(s->blk->conf.bs); +bdrv_io_unplug(s->conf->conf.bs); } /* Context: QEMU global mutex held */ -void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, +void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf, VirtIOBlockDataPlane **dataplane, Error **errp) { @@ -148,7 +148,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, *dataplane = NULL; -if (!blk->data_plane && !blk->iothread) { +if (!conf->data_plane && !conf->iothread) { return; } @@ -163,7 +163,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, /* If dataplane is (re-)enabled while the guest is running there could be * block jobs that can conflict. */ -if (bdrv_op_is_blocked(blk->conf.bs, BLOCK_OP_TYPE_DATAPLANE, &local_err)) { +if (bdrv_op_is_blocked(conf->conf.bs, BLOCK_OP_TYPE_DATAPLANE, + &local_err)) { error_setg(errp, "cannot start dataplane thread: %s", error_get_pretty(local_err)); error_free(local_err); @@ -172,10 +173,10 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, s = g_new0(VirtIOBlockDataPlane, 1); s->vdev = vdev; -s->blk = blk; +s->conf = conf; -if (blk->iothread) { -s->iothread = blk->iothread; +if (conf->iothread) { +s->iothread = conf->iothread; object_ref(OBJECT(s->iothread)); } else { /* Create per-device IOThread if none specified. This is for @@ -192,9 +193,9 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, s->bh = aio_bh_new(s->ctx, notify_guest_bh, s); error_setg(&s->blocker, "block device is in use by data plane"); -bdrv_op_block_all(blk->conf.bs, s->blocker); -bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_RESIZE, s->blocker); -bdrv_op_unblock(blk->conf.bs, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker); +bdrv_op_block_all(conf->conf.bs, s->blocker); +bdrv_op_unblock(conf->conf.bs, BLOCK_OP_TYPE_RESIZE, s->blocker); +bdrv_op_unblock(conf->conf.bs, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker); *dataplane = s; } @@ -207,7 +208,7 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s) } virtio_blk_data_plane_stop(s); -bdrv_op_unblock_all(s->blk->conf.bs, s->blocker); +bdrv_op_unblock_all(s->conf->conf.bs, s->blocker); error_free(s->blocker); object_unref(OBJECT(s->iothread)); qemu_bh_delete(s->bh); @@ -262,7 +263,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s) s->started = true; trace_virtio_blk_data_plane_start(s); -bdrv_set_aio_context(s->blk->conf.bs, s->ctx); +bdrv_set_aio_context(s->conf->conf.bs, s->ctx); /* Kick right away to begin processing requests already in vring */ event_notifier_set(virtio_queue_get_host_notifier(vq)); @@ -308,7 +309,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) aio_set_event_notifier(s->ctx, &s->host_notifier, NULL); /* Drain and switch bs back to the QEMU main loop */ -bdrv_set_aio_context(s->blk->conf.bs, qemu_get_aio_context()); +bdrv_set_aio_context(s->con
[Qemu-devel] [PATCH v5 13/23] virtio-blk: Drop redundant VirtIOBlock member conf
Commit 12c5674 turned it into a pointer to member blk.conf. Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- hw/block/virtio-blk.c | 28 ++-- include/hw/virtio/virtio-blk.h | 1 - 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 45e0c8f..5dc1b29 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -296,7 +296,7 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev, if (sector & dev->sector_mask) { return false; } -if (size % dev->conf->logical_block_size) { +if (size % dev->blk.conf.logical_block_size) { return false; } bdrv_get_geometry(dev->bs, &total_sectors); @@ -513,19 +513,20 @@ static void virtio_blk_reset(VirtIODevice *vdev) static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) { VirtIOBlock *s = VIRTIO_BLK(vdev); +BlockConf *conf = &s->blk.conf; struct virtio_blk_config blkcfg; uint64_t capacity; -int blk_size = s->conf->logical_block_size; +int blk_size = conf->logical_block_size; bdrv_get_geometry(s->bs, &capacity); memset(&blkcfg, 0, sizeof(blkcfg)); virtio_stq_p(vdev, &blkcfg.capacity, capacity); virtio_stl_p(vdev, &blkcfg.seg_max, 128 - 2); -virtio_stw_p(vdev, &blkcfg.cylinders, s->conf->cyls); +virtio_stw_p(vdev, &blkcfg.cylinders, conf->cyls); virtio_stl_p(vdev, &blkcfg.blk_size, blk_size); -virtio_stw_p(vdev, &blkcfg.min_io_size, s->conf->min_io_size / blk_size); -virtio_stw_p(vdev, &blkcfg.opt_io_size, s->conf->opt_io_size / blk_size); -blkcfg.heads = s->conf->heads; +virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size); +virtio_stw_p(vdev, &blkcfg.opt_io_size, conf->opt_io_size / blk_size); +blkcfg.heads = conf->heads; /* * We must ensure that the block device capacity is a multiple of * the logical block size. If that is not the case, let's use @@ -537,13 +538,13 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) * divided by 512 - instead it is the amount of blk_size blocks * per track (cylinder). */ -if (bdrv_getlength(s->bs) / s->conf->heads / s->conf->secs % blk_size) { -blkcfg.sectors = s->conf->secs & ~s->sector_mask; +if (bdrv_getlength(s->bs) / conf->heads / conf->secs % blk_size) { +blkcfg.sectors = conf->secs & ~s->sector_mask; } else { -blkcfg.sectors = s->conf->secs; +blkcfg.sectors = conf->secs; } blkcfg.size_max = 0; -blkcfg.physical_block_exp = get_physical_block_exp(s->conf); +blkcfg.physical_block_exp = get_physical_block_exp(conf); blkcfg.alignment_offset = 0; blkcfg.wce = bdrv_enable_write_cache(s->bs); memcpy(config, &blkcfg, sizeof(struct virtio_blk_config)); @@ -746,9 +747,8 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) sizeof(struct virtio_blk_config)); s->bs = blk->conf.bs; -s->conf = &blk->conf; s->rq = NULL; -s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; +s->sector_mask = (s->blk.conf.logical_block_size / BDRV_SECTOR_SIZE) - 1; s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output); s->complete_request = virtio_blk_complete_request; @@ -765,11 +765,11 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) register_savevm(dev, "virtio-blk", virtio_blk_id++, 2, virtio_blk_save, virtio_blk_load, s); bdrv_set_dev_ops(s->bs, &virtio_block_ops, s); -bdrv_set_guest_block_size(s->bs, s->conf->logical_block_size); +bdrv_set_guest_block_size(s->bs, s->blk.conf.logical_block_size); bdrv_iostatus_enable(s->bs); -add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0"); +add_boot_device_path(s->blk.conf.bootindex, dev, "/disk@0,0"); } static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp) diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index f3853f2..0f4323d 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -125,7 +125,6 @@ typedef struct VirtIOBlock { VirtQueue *vq; void *rq; QEMUBH *bh; -BlockConf *conf; VirtIOBlkConf blk; unsigned short sector_mask; bool original_wce; -- 1.9.3
[Qemu-devel] [PATCH v5 04/23] block: Connect BlockBackend and DriveInfo
Make the BlockBackend own the DriveInfo. Change blockdev_init() to return the BlockBackend instead of the DriveInfo. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block.c | 2 -- block/block-backend.c | 38 ++ blockdev.c| 69 --- include/sysemu/blockdev.h | 4 +++ 4 files changed, 77 insertions(+), 36 deletions(-) diff --git a/block.c b/block.c index a25fae9..d6ab0c7 100644 --- a/block.c +++ b/block.c @@ -29,7 +29,6 @@ #include "qemu/module.h" #include "qapi/qmp/qjson.h" #include "sysemu/sysemu.h" -#include "sysemu/blockdev.h"/* FIXME layering violation */ #include "qemu/notify.h" #include "block/coroutine.h" #include "block/qapi.h" @@ -2131,7 +2130,6 @@ static void bdrv_delete(BlockDriverState *bs) /* remove from list, if necessary */ bdrv_make_anon(bs); -drive_info_del(drive_get_by_blockdev(bs)); g_free(bs); } diff --git a/block/block-backend.c b/block/block-backend.c index a12215a..edaf73c 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -12,11 +12,13 @@ #include "sysemu/block-backend.h" #include "block/block_int.h" +#include "sysemu/blockdev.h" struct BlockBackend { char *name; int refcnt; BlockDriverState *bs; +DriveInfo *legacy_dinfo; QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */ }; @@ -87,6 +89,7 @@ static void blk_delete(BlockBackend *blk) QTAILQ_REMOVE(&blk_backends, blk, link); } g_free(blk->name); +drive_info_del(blk->legacy_dinfo); g_free(blk); } @@ -167,6 +170,41 @@ BlockDriverState *blk_bs(BlockBackend *blk) } /* + * Return @blk's DriveInfo if any, else null. + */ +DriveInfo *blk_legacy_dinfo(BlockBackend *blk) +{ +return blk->legacy_dinfo; +} + +/* + * Set @blk's DriveInfo to @dinfo, and return it. + * @blk must not have a DriveInfo set already. + * No other BlockBackend may have the same DriveInfo set. + */ +DriveInfo *blk_set_legacy_dinfo(BlockBackend *blk, DriveInfo *dinfo) +{ +assert(!blk->legacy_dinfo); +return blk->legacy_dinfo = dinfo; +} + +/* + * Return the BlockBackend with DriveInfo @dinfo. + * It must exist. + */ +BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo) +{ +BlockBackend *blk; + +QTAILQ_FOREACH(blk, &blk_backends, link) { +if (blk->legacy_dinfo == dinfo) { +return blk; +} +} +abort(); +} + +/* * Hide @blk. * @blk must not have been hidden already. * Make attached BlockDriverState, if any, anonymous. diff --git a/blockdev.c b/blockdev.c index 494714b..3a6bbe9 100644 --- a/blockdev.c +++ b/blockdev.c @@ -47,8 +47,6 @@ #include "trace.h" #include "sysemu/arch_init.h" -static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives); - static const char *const if_name[IF_COUNT] = { [IF_NONE] = "none", [IF_IDE] = "ide", @@ -89,7 +87,8 @@ static const int if_max_devs[IF_COUNT] = { */ void blockdev_mark_auto_del(BlockDriverState *bs) { -DriveInfo *dinfo = drive_get_by_blockdev(bs); +BlockBackend *blk = bs->blk; +DriveInfo *dinfo = blk_legacy_dinfo(blk); if (dinfo && !dinfo->enable_auto_del) { return; @@ -105,7 +104,8 @@ void blockdev_mark_auto_del(BlockDriverState *bs) void blockdev_auto_del(BlockDriverState *bs) { -DriveInfo *dinfo = drive_get_by_blockdev(bs); +BlockBackend *blk = bs->blk; +DriveInfo *dinfo = blk_legacy_dinfo(blk); if (dinfo && dinfo->auto_del) { drive_del(dinfo); @@ -153,15 +153,15 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit) { +BlockBackend *blk; DriveInfo *dinfo; -/* seek interface, bus and unit */ - -QTAILQ_FOREACH(dinfo, &drives, next) { -if (dinfo->type == type && - dinfo->bus == bus && - dinfo->unit == unit) +for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { +dinfo = blk_legacy_dinfo(blk); +if (dinfo && dinfo->type == type +&& dinfo->bus == bus && dinfo->unit == unit) { return dinfo; +} } return NULL; @@ -177,13 +177,15 @@ DriveInfo *drive_get_by_index(BlockInterfaceType type, int index) int drive_get_max_bus(BlockInterfaceType type) { int max_bus; +BlockBackend *blk; DriveInfo *dinfo; max_bus = -1; -QTAILQ_FOREACH(dinfo, &drives, next) { -if(dinfo->type == type && - dinfo->bus > max_bus) +for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { +dinfo = blk_legacy_dinfo(blk); +if (dinfo && dinfo->type == type && dinfo->bus > max_bus) { max_bus = dinfo->bus; +} } return max_bus; } @@ -200,11 +202,11 @@ DriveInfo *drive_get_next(BlockInterfaceType type) DriveInfo *drive_get_by_blockdev(BlockDriverState *bs) { -DriveInfo *dinfo
[Qemu-devel] [PATCH v5 02/23] block: New BlockBackend
A block device consists of a frontend device model and a backend. A block backend has a tree of block drivers doing the actual work. The tree is managed by the block layer. We currently use a single abstraction BlockDriverState both for tree nodes and the backend as a whole. Drawbacks: * Its API includes both stuff that makes sense only at the block backend level (root of the tree) and stuff that's only for use within the block layer. This makes the API bigger and more complex than necessary. Moreover, it's not obvious which interfaces are meant for device models, and which really aren't. * Since device models keep a reference to their backend, the backend object can't just be destroyed. But for media change, we need to replace the tree. Our solution is to make the BlockDriverState generic, with actual driver state in a separate object, pointed to by member opaque. That lets us replace the tree by deinitializing and reinitializing its root. This special need of the root makes the data structure awkward everywhere in the tree. The general plan is to separate the APIs into "block backend", for use by device models, monitor and whatever other code dealing with block backends, and "block driver", for use by the block layer and whatever other code (if any) dealing with trees and tree nodes. Code dealing with block backends, device models in particular, should become completely oblivious of BlockDriverState. This should let us clean up both APIs, and the tree data structures. This commit is a first step. It creates a minimal "block backend" API: type BlockBackend and functions to create, destroy and find them. BlockBackend objects are created and destroyed exactly when root BlockDriverState objects are created and destroyed. "Root" in the sense of "in bdrv_states". They're not yet used for anything; that'll come shortly. BlockBackend is reference-counted. Its reference count never exceeds one so far, but that's going to change. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block/Makefile.objs| 2 +- block/block-backend.c | 120 + blockdev.c | 13 - hw/block/xen_disk.c| 11 include/qemu/typedefs.h| 1 + include/sysemu/block-backend.h | 26 + qemu-img.c | 70 +--- qemu-io.c | 8 +++ qemu-nbd.c | 5 +- 9 files changed, 245 insertions(+), 11 deletions(-) create mode 100644 block/block-backend.c create mode 100644 include/sysemu/block-backend.h diff --git a/block/Makefile.objs b/block/Makefile.objs index a833ed5..27911b6 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -5,7 +5,7 @@ block-obj-y += qed-check.o block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o block-obj-$(CONFIG_QUORUM) += quorum.o block-obj-y += parallels.o blkdebug.o blkverify.o -block-obj-y += snapshot.o qapi.o +block-obj-y += block-backend.o snapshot.o qapi.o block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o block-obj-$(CONFIG_POSIX) += raw-posix.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o diff --git a/block/block-backend.c b/block/block-backend.c new file mode 100644 index 000..e89caa9 --- /dev/null +++ b/block/block-backend.c @@ -0,0 +1,120 @@ +/* + * QEMU Block backends + * + * Copyright (C) 2014 Red Hat, Inc. + * + * Authors: + * Markus Armbruster , + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 + * or later. See the COPYING.LIB file in the top-level directory. + */ + +#include "sysemu/block-backend.h" +#include "block/block_int.h" + +struct BlockBackend { +char *name; +int refcnt; +QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */ +}; + +/* All the BlockBackends */ +static QTAILQ_HEAD(, BlockBackend) blk_backends = +QTAILQ_HEAD_INITIALIZER(blk_backends); + +/* + * Create a new BlockBackend with @name, with a reference count of one. + * @name must not be null or empty. + * Fail if a BlockBackend with this name already exists. + * Store an error through @errp on failure, unless it's null. + * Return the new BlockBackend on success, null on failure. + */ +BlockBackend *blk_new(const char *name, Error **errp) +{ +BlockBackend *blk; + +assert(name && name[0]); +if (blk_by_name(name)) { +error_setg(errp, "Device with id '%s' already exists", name); +return NULL; +} + +blk = g_new0(BlockBackend, 1); +blk->name = g_strdup(name); +blk->refcnt = 1; +QTAILQ_INSERT_TAIL(&blk_backends, blk, link); +return blk; +} + +static void blk_delete(BlockBackend *blk) +{ +assert(!blk->refcnt); +QTAILQ_REMOVE(&blk_backends, blk, link); +g_free(blk->name); +g_free(blk); +} + +/* + * Increment @blk's reference count. + * @blk must not be null. + */ +void blk_ref(BlockBackend *blk) +{ +blk->refcnt++; +} + +/* + * Decrement @blk's reference coun
[Qemu-devel] [PATCH v5 03/23] block: Connect BlockBackend to BlockDriverState
Convenience function blk_new_with_bs() creates a BlockBackend with its BlockDriverState. Callers have to unref both. The commit after next will relieve them of the need to unref the BlockDriverState. Complication: due to the silly way drive_del works, we need a way to hide a BlockBackend, just like bdrv_make_anon(). To emphasize its "special" status, give the function a suitably off-putting name: blk_hide_on_behalf_of_do_drive_del(). Unfortunately, hiding turns the BlockBackend's name into the empty string. Can't avoid that without breaking the blk->bs->device_name equals blk->name invariant. The patch adds a memory leak: drive_del while a device model is connected leaks the BlockBackend. Avoiding the leak here is rather hairy, but it'll become straightforward shortly, so I mark it FIXME in the code now, and plug it when it's easy. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block.c| 12 ++-- block/block-backend.c | 71 ++- blockdev.c | 18 +++--- hw/block/xen_disk.c| 8 +-- include/block/block_int.h | 2 + include/sysemu/block-backend.h | 5 ++ qemu-img.c | 125 +++-- qemu-io.c | 4 +- qemu-nbd.c | 4 +- 9 files changed, 156 insertions(+), 93 deletions(-) diff --git a/block.c b/block.c index 3905c32..a25fae9 100644 --- a/block.c +++ b/block.c @@ -2025,6 +2025,8 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name), bs_src->device_name); bs_dest->device_list = bs_src->device_list; +bs_dest->blk = bs_src->blk; + memcpy(bs_dest->op_blockers, bs_src->op_blockers, sizeof(bs_dest->op_blockers)); } @@ -2037,7 +2039,7 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, * This will modify the BlockDriverState fields, and swap contents * between bs_new and bs_old. Both bs_new and bs_old are modified. * - * bs_new is required to be anonymous. + * bs_new must be nameless and not attached to a BlockBackend. * * This function does not create any image files. */ @@ -2056,8 +2058,9 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) QTAILQ_REMOVE(&graph_bdrv_states, bs_old, node_list); } -/* bs_new must be anonymous and shouldn't have anything fancy enabled */ +/* bs_new must be nameless and shouldn't have anything fancy enabled */ assert(bs_new->device_name[0] == '\0'); +assert(!bs_new->blk); assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(bs_new->job == NULL); assert(bs_new->dev == NULL); @@ -2073,8 +2076,9 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) bdrv_move_feature_fields(bs_old, bs_new); bdrv_move_feature_fields(bs_new, &tmp); -/* bs_new shouldn't be in bdrv_states even after the swap! */ +/* bs_new must remain nameless and unattached */ assert(bs_new->device_name[0] == '\0'); +assert(!bs_new->blk); /* Check a few fields that should remain attached to the device */ assert(bs_new->dev == NULL); @@ -2101,7 +2105,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) * This will modify the BlockDriverState fields, and swap contents * between bs_new and bs_top. Both bs_new and bs_top are modified. * - * bs_new is required to be anonymous. + * bs_new must be nameless and not attached to a BlockBackend. * * This function does not create any image files. */ diff --git a/block/block-backend.c b/block/block-backend.c index e89caa9..a12215a 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -16,10 +16,11 @@ struct BlockBackend { char *name; int refcnt; +BlockDriverState *bs; QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */ }; -/* All the BlockBackends */ +/* All the BlockBackends (except for hidden ones) */ static QTAILQ_HEAD(, BlockBackend) blk_backends = QTAILQ_HEAD_INITIALIZER(blk_backends); @@ -47,10 +48,44 @@ BlockBackend *blk_new(const char *name, Error **errp) return blk; } +/* + * Create a new BlockBackend with a new BlockDriverState attached. + * Both have a reference count of one. Caller owns *both* references. + * TODO Let caller own only the BlockBackend reference + * Otherwise just like blk_new(), which see. + */ +BlockBackend *blk_new_with_bs(const char *name, Error **errp) +{ +BlockBackend *blk; +BlockDriverState *bs; + +blk = blk_new(name, errp); +if (!blk) { +return NULL; +} + +bs = bdrv_new_root(name, errp); +if (!bs) { +blk_unref(blk); +return NULL; +} + +blk->bs = bs; +bs->blk = blk; +return blk; +} + static void blk_delete(BlockBackend *blk) { assert(!blk->refcnt); -QTAILQ_REMOVE(&blk_backends, blk, link); +if (blk->bs) { +
[Qemu-devel] [PATCH v5 00/23] Split BlockBackend off BDS with an axe
My last attempt got bogged down because I tried to do a reasonably complete job, and the complexity proved more than I could handle with the limited amount of uninterrupted time available. This time, I'm cutting BlockBackend off with an axe, leaving most of the work for later. Done in this series already: * Introduce a BlockBackend type, and lift up BlockDriverState's device_name, device_list, dev, dev_ops, dev_opaque. Much more remains to be lifted. * Make BlockBackend own the DriveInfo. * Wean hw/ off BlockDriverState, with two small exceptions. * Fix blockdev-add not to create a bogus IDE drive (0,0). * Take a few baby steps towards use of BlockBackend in monitor command code where appropriate. Coming soon, hopefully: * QMP command blockdev-del * blockdev-add accepts node-name without id at top level * Lift up more stuff * More BlockBackend use in monitor command code Depends on my [PATCH] block: Drop superfluous conditionals around qemu_opts_del() [PATCH] util: Emancipate id_wellformed() from QemuOpts Tested on top of Kevin's [PATCH 0/2] make check-block/qemu-iotests fixes I know the diffstat looks intimidating. I tried very hard to split the patches so that the bigger ones do just one simple thing, and mostly mechanically. v5: * Rebased - Straightforward conflicts in PATCH 10, R-bys retained - Semantic conflicts in PATCH 15 under hw/scsi/, R-by dropped Straightforward conflicts elsewhere * [PATCH 03/23] block: Connect BlockBackend to BlockDriverState - Commit message tweak regarding the FIXME * [PATCH 11/23] block: Rename BlockDriverAIOCB* to BlockAIOCB* - Clean up indentation after rename [Max] Ripples into PATCH 12 and PATCH 15 harmlessly - Rename in docs/blkdebug.txt, too [Max] * [PATCH 12/23] block: Rename BlockDriverCompletionFunc to BlockCompletionFunc - Rename in docs/blkdebug.txt, too [Max] v4: * Rebased, some R-bys dropped due to non-trivial conflicts * Spin off dropping the conditional around qemu_opts_del() [Kevin] * [PATCH 02/23] block: New BlockBackend - Fix license in block-backend.h [Kevin] - Avoid some temporary BB leaks [Kevin] * [PATCH 03/23] block: Connect BlockBackend to BlockDriverState - Move commit message paragraph on strong references to PATCH 06, because that's where they actually become strong - Update bdrv_move_feature_fields() [Kevin] - Ripple effects from PATCH 02's leak avoidance * [PATCH 04/23] block: Connect BlockBackend and DriveInfo - Use abort() instead of assert(0) to avoid hypothetical compiler warnings [Kevin] - Simplify the "is this the BB on top of this BDS" test in drive_get_by_blockdev() [Kevin] - Ripple effects from PATCH 02's leak avoidance - Avoid crash on -drive format=help [Max] - Drop a FIXME here instead of in PATCH 06, because this one actually fixes it[Kevin] * [PATCH 08/23] block: Eliminate BlockDriverState member device_name[] - Delay code motion from bdrv_new_root() to caller until the next patch * [PATCH 09/23] block: Merge BlockBackend and BlockDriverState name spaces - See PATCH 08 - Add TODO comments to bdrv_find() and bdrv_get_device_name() * [PATCH 11/23] block: Rename BlockDriverAIOCB* to BlockAIOCB* - Split rename of BlockDriverCompletionFunc into its own PATCH 12 * [PATCH 12/23] block: Rename BlockDriverCompletionFunc to BlockCompletionFunc - New, split off PATCH 11 * [PATCH 13/23] virtio-blk: Drop redundant VirtIOBlock member conf - Use conf instead of &s->blk.conf [Max] * [PATCH 14/23] virtio-blk: Rename VirtIOBlkConf variables to conf - Trivial ripple effect from change to PATCH 13 * [PATCH 15/23] hw: Convert from BlockDriverState to BlockBackend, mostly - Explain in commit message why includes are changed [Max] * [PATCH 19/23] blockdev: Fix blockdev-add not to create DriveInfo - Fix temporary breakage by squashing in the next patch [Fam] - Document legacy_dinfo exists only when created by drive_new() [Max] * [PATCH 22/23] block: Lift device model API into BlockBackend - More verbose commit message [Benoît] * [PATCH 23/23] block: Make device model's references to BlockBackend strong - Code rearranged slightly for robustness [Max] v3: * Rebased * A few comments here and there [Benoît] * PATCH 02-04: Drop flawed attempt to avoid leaking BB on drive_del while a device model is connected [Flaw pointed out by Max] * PATCH 02: Check blk_by_name()'s precondition with assert() [Benoît] * PATCH 03: blk_new_with_bs() in qemu-nbd.c [Max, Benoît] * PATCH 10: Cover hw/arm/virt.c (semantic conflict) * PATCH 14: Plenty of conflicts with Benoît's "Extract block accounting statistic code in it's own module" series v2: * General - Improved function comments [Kevin, Benoît] - Avoiding temporary badness around drive_del affects several patches up to PATCH 14/23, but by then the solution is basically v1's again, plus a simple bug fix. * [PATCH 01/23] block: Split bdrv_new_root() off bdrv_new() - Call it bdrv_new_ro
[Qemu-devel] [PATCH v5 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo()
The patch is big, but all it really does is replacing dinfo->bdrv by blk_bs(blk_by_legacy_dinfo(dinfo)) The replacement is repetitive, but the conversion of device models to BlockBackend is imminent, and will shorten it to just blk_legacy_dinfo(dinfo). Line wrapping muddies the waters a bit. I also omit tests whether dinfo->bdrv is null, because it never is. Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- blockdev.c | 3 +-- hw/arm/collie.c | 9 + hw/arm/gumstix.c | 5 +++-- hw/arm/mainstone.c | 8 hw/arm/musicpal.c| 11 ++- hw/arm/nseries.c | 6 -- hw/arm/omap1.c | 4 +++- hw/arm/omap2.c | 4 +++- hw/arm/omap_sx1.c| 9 + hw/arm/pxa2xx.c | 7 +-- hw/arm/spitz.c | 4 +++- hw/arm/versatilepb.c | 4 +++- hw/arm/vexpress.c| 4 +++- hw/arm/virt.c| 4 +++- hw/arm/xilinx_zynq.c | 4 +++- hw/arm/z2.c | 7 --- hw/block/fdc.c | 16 +++- hw/block/m25p80.c| 5 +++-- hw/block/xen_disk.c | 2 +- hw/cris/axis_dev88.c | 3 ++- hw/display/tc6393xb.c| 4 +++- hw/i386/pc_sysfw.c | 3 ++- hw/ide/piix.c| 6 -- hw/ide/qdev.c| 4 +++- hw/isa/pc87312.c | 7 +-- hw/lm32/lm32_boards.c| 13 +++-- hw/lm32/milkymist.c | 7 --- hw/microblaze/petalogix_ml605_mmu.c | 5 +++-- hw/microblaze/petalogix_s3adsp1800_mmu.c | 5 +++-- hw/mips/mips_malta.c | 4 +++- hw/mips/mips_r4k.c | 5 +++-- hw/pci/pci-hotplug-old.c | 10 +++--- hw/ppc/ppc405_boards.c | 25 - hw/ppc/spapr.c | 4 +++- hw/ppc/virtex_ml507.c| 5 +++-- hw/scsi/scsi-bus.c | 5 +++-- hw/sd/milkymist-memcard.c| 7 +-- hw/sd/pl181.c| 3 ++- hw/sd/sdhci.c| 3 ++- hw/sd/ssi-sd.c | 3 ++- hw/sh4/r2d.c | 5 +++-- hw/usb/dev-storage.c | 4 +++- hw/xtensa/xtfpga.c | 4 +++- include/sysemu/blockdev.h| 1 - 44 files changed, 167 insertions(+), 94 deletions(-) diff --git a/blockdev.c b/blockdev.c index 09ce9fa..b3d2a22 100644 --- a/blockdev.c +++ b/blockdev.c @@ -472,7 +472,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, dinfo = g_malloc0(sizeof(*dinfo)); dinfo->id = g_strdup(qemu_opts_id(opts)); -dinfo->bdrv = bs; blk_set_legacy_dinfo(blk, dinfo); if (!file || !*file) { @@ -502,7 +501,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, QINCREF(bs_opts); ret = bdrv_open(&bs, file, NULL, bs_opts, bdrv_flags, drv, &error); -assert(bs == dinfo->bdrv); +assert(bs == blk_bs(blk)); if (ret < 0) { error_setg(errp, "could not open disk image %s: %s", diff --git a/hw/arm/collie.c b/hw/arm/collie.c index ed7851f..0247290 100644 --- a/hw/arm/collie.c +++ b/hw/arm/collie.c @@ -15,6 +15,7 @@ #include "strongarm.h" #include "hw/arm/arm.h" #include "hw/block/flash.h" +#include "sysemu/block-backend.h" #include "sysemu/blockdev.h" #include "exec/address-spaces.h" @@ -41,13 +42,13 @@ static void collie_init(MachineState *machine) dinfo = drive_get(IF_PFLASH, 0, 0); pflash_cfi01_register(SA_CS0, NULL, "collie.fl1", 0x0200, -dinfo ? dinfo->bdrv : NULL, (64 * 1024), -512, 4, 0x00, 0x00, 0x00, 0x00, 0); +dinfo ? blk_bs(blk_by_legacy_dinfo(dinfo)) : NULL, +(64 * 1024), 512, 4, 0x00, 0x00, 0x00, 0x00, 0); dinfo = drive_get(IF_PFLASH, 0, 1); pflash_cfi01_register(SA_CS1, NULL, "collie.fl2", 0x0200, -dinfo ? dinfo->bdrv : NULL, (64 * 1024), -512, 4, 0x00, 0x00, 0x00, 0x00, 0); +dinfo ? blk_bs(blk_by_legacy_dinfo(dinfo)) : NULL, +(64 * 1024), 512, 4, 0x00, 0x00, 0x00, 0x00, 0); sysbus_create_simple("scoop", 0x4080, NULL); diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c index 3f8465e..49f9339 100644 --- a/hw/arm/gumstix.c +++ b/hw/arm/gumstix.c @@ -40,6 +40,7 @@ #include "hw/block/flash.h" #include "hw/de
[Qemu-devel] [PATCH v5 09/23] block: Merge BlockBackend and BlockDriverState name spaces
BlockBackend's name space is separate only to keep the initial patches simple. Time to merge the two. Retain bdrv_find() and bdrv_get_device_name() for now, to keep this series manageable. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block.c | 48 block/block-backend.c | 17 +++-- include/block/block.h | 2 +- 3 files changed, 24 insertions(+), 43 deletions(-) diff --git a/block.c b/block.c index fd57cc0..b075669 100644 --- a/block.c +++ b/block.c @@ -335,31 +335,9 @@ void bdrv_register(BlockDriver *bdrv) QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list); } -/* create a new block device (by default it is empty) */ -BlockDriverState *bdrv_new_root(const char *device_name, Error **errp) +BlockDriverState *bdrv_new_root(void) { -BlockDriverState *bs; - -assert(*device_name); - -if (*device_name && !id_wellformed(device_name)) { -error_setg(errp, "Invalid device name"); -return NULL; -} - -if (bdrv_find(device_name)) { -error_setg(errp, "Device with id '%s' already exists", - device_name); -return NULL; -} -if (bdrv_find_node(device_name)) { -error_setg(errp, - "Device name '%s' conflicts with an existing node name", - device_name); -return NULL; -} - -bs = bdrv_new(); +BlockDriverState *bs = bdrv_new(); QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); return bs; @@ -883,7 +861,7 @@ static void bdrv_assign_node_name(BlockDriverState *bs, } /* takes care of avoiding namespaces collisions */ -if (bdrv_find(node_name)) { +if (blk_by_name(node_name)) { error_setg(errp, "node-name=%s is conflicting with a device id", node_name); return; @@ -3817,16 +3795,12 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name), } /* This function is to find block backend bs */ +/* TODO convert callers to blk_by_name(), then remove */ BlockDriverState *bdrv_find(const char *name) { -BlockDriverState *bs; +BlockBackend *blk = blk_by_name(name); -QTAILQ_FOREACH(bs, &bdrv_states, device_list) { -if (!strcmp(name, bdrv_get_device_name(bs))) { -return bs; -} -} -return NULL; +return blk ? blk_bs(blk) : NULL; } /* This function is to find a node in the bs graph */ @@ -3865,13 +3839,14 @@ BlockDriverState *bdrv_lookup_bs(const char *device, const char *node_name, Error **errp) { -BlockDriverState *bs = NULL; +BlockBackend *blk; +BlockDriverState *bs; if (device) { -bs = bdrv_find(device); +blk = blk_by_name(device); -if (bs) { -return bs; +if (blk) { +return blk_bs(blk); } } @@ -3908,6 +3883,7 @@ BlockDriverState *bdrv_next(BlockDriverState *bs) return QTAILQ_NEXT(bs, device_list); } +/* TODO check what callers really want: bs->node_name or blk_name() */ const char *bdrv_get_device_name(const BlockDriverState *bs) { return bs->blk ? blk_name(bs->blk) : ""; diff --git a/block/block-backend.c b/block/block-backend.c index 6236b5b..fb91680 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -40,10 +40,20 @@ BlockBackend *blk_new(const char *name, Error **errp) BlockBackend *blk; assert(name && name[0]); +if (!id_wellformed(name)) { +error_setg(errp, "Invalid device name"); +return NULL; +} if (blk_by_name(name)) { error_setg(errp, "Device with id '%s' already exists", name); return NULL; } +if (bdrv_find_node(name)) { +error_setg(errp, + "Device name '%s' conflicts with an existing node name", + name); +return NULL; +} blk = g_new0(BlockBackend, 1); blk->name = g_strdup(name); @@ -66,12 +76,7 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp) return NULL; } -bs = bdrv_new_root(name, errp); -if (!bs) { -blk_unref(blk); -return NULL; -} - +bs = bdrv_new_root(); blk->bs = bs; bs->blk = blk; return blk; diff --git a/include/block/block.h b/include/block/block.h index 3880e05..3dc7c56 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -204,7 +204,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name, int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts, Error **errp); int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp); -BlockDriverState *bdrv_new_root(const char *device_name, Error **errp); +BlockDriverState *bdrv_new_root(void); BlockDriverState *bdrv_new(void); void bdrv_make_anon(BlockDriverState *bs); void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_o
[Qemu-devel] [PATCH v5 18/23] blockdev: Drop superfluous DriveInfo member id
Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- block/block-backend.c | 1 - blockdev.c| 3 +-- include/sysemu/blockdev.h | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 58ae634..a3e613b 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -106,7 +106,6 @@ static void drive_info_del(DriveInfo *dinfo) return; } qemu_opts_del(dinfo->opts); -g_free(dinfo->id); g_free(dinfo->serial); g_free(dinfo); } diff --git a/blockdev.c b/blockdev.c index f4f9e04..b102fe0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -458,7 +458,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, } dinfo = g_malloc0(sizeof(*dinfo)); -dinfo->id = g_strdup(qemu_opts_id(opts)); blk_set_legacy_dinfo(blk, dinfo); if (!file || !*file) { @@ -492,7 +491,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, if (ret < 0) { error_setg(errp, "could not open disk image %s: %s", - file ?: dinfo->id, error_get_pretty(error)); + file ?: blk_name(blk), error_get_pretty(error)); error_free(error); goto err; } diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index f66b89a..27a40d5 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -30,7 +30,6 @@ typedef enum { } BlockInterfaceType; struct DriveInfo { -char *id; const char *devaddr; BlockInterfaceType type; int bus; -- 1.9.3
[Qemu-devel] [PATCH v5 06/23] block: Make BlockBackend own its BlockDriverState
On BlockBackend destruction, unref its BlockDriverState. Replaces the callers' unrefs. This turns the pointer from BlockBackend to BlockDriverState into a strong reference, managed with bdrv_ref() / bdrv_unref(). The back-pointer remains weak. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- block/block-backend.c | 6 ++ blockdev.c| 7 ++- hw/block/xen_disk.c | 6 +++--- qemu-img.c| 35 +-- qemu-io.c | 5 - qemu-nbd.c| 1 - 6 files changed, 8 insertions(+), 52 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index d4bdd48..6236b5b 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -54,8 +54,6 @@ BlockBackend *blk_new(const char *name, Error **errp) /* * Create a new BlockBackend with a new BlockDriverState attached. - * Both have a reference count of one. Caller owns *both* references. - * TODO Let caller own only the BlockBackend reference * Otherwise just like blk_new(), which see. */ BlockBackend *blk_new_with_bs(const char *name, Error **errp) @@ -83,7 +81,9 @@ static void blk_delete(BlockBackend *blk) { assert(!blk->refcnt); if (blk->bs) { +assert(blk->bs->blk == blk); blk->bs->blk = NULL; +bdrv_unref(blk->bs); blk->bs = NULL; } /* Avoid double-remove after blk_hide_on_behalf_of_do_drive_del() */ @@ -119,8 +119,6 @@ void blk_ref(BlockBackend *blk) * Decrement @blk's reference count. * If this drops it to zero, destroy @blk. * For convenience, do nothing if @blk is null. - * Does *not* touch the attached BlockDriverState's reference count. - * TODO Decrement it! */ void blk_unref(BlockBackend *blk) { diff --git a/blockdev.c b/blockdev.c index 7fcd26c..07321b1 100644 --- a/blockdev.c +++ b/blockdev.c @@ -108,7 +108,7 @@ void blockdev_auto_del(BlockDriverState *bs) DriveInfo *dinfo = blk_legacy_dinfo(blk); if (dinfo && dinfo->auto_del) { -drive_del(dinfo); +blk_unref(blk); } } @@ -219,7 +219,6 @@ static void bdrv_format_print(void *opaque, const char *name) void drive_del(DriveInfo *dinfo) { -bdrv_unref(dinfo->bdrv); blk_unref(blk_by_legacy_dinfo(dinfo)); } @@ -522,7 +521,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, return blk; err: -bdrv_unref(bs); blk_unref(blk); early_err: qemu_opts_del(opts); @@ -1806,7 +1804,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT); } else { -drive_del(dinfo); +blk_unref(blk); } aio_context_release(aio_context); @@ -2543,7 +2541,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) } if (bdrv_key_required(blk_bs(blk))) { -bdrv_unref(blk_bs(blk)); blk_unref(blk); error_setg(errp, "blockdev-add doesn't support encrypted devices"); goto fail; diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 0022083..feb227f 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -872,7 +872,6 @@ static int blk_connect(struct XenDevice *xendev) xen_be_printf(&blkdev->xendev, 0, "error: %s\n", error_get_pretty(local_err)); error_free(local_err); -bdrv_unref(blkdev->bs); blk_unref(blk); blkdev->bs = NULL; return -1; @@ -888,7 +887,9 @@ static int blk_connect(struct XenDevice *xendev) } /* blkdev->bs is not create by us, we get a reference * so we can bdrv_unref() unconditionally */ -bdrv_ref(blkdev->bs); +/* Except we don't bdrv_unref() anymore, we blk_unref(). + * Conditionally, because we can't easily blk_ref() here. + * TODO Clean this up! */ } bdrv_attach_dev_nofail(blkdev->bs, blkdev); blkdev->file_size = bdrv_getlength(blkdev->bs); @@ -988,7 +989,6 @@ static void blk_disconnect(struct XenDevice *xendev) if (blkdev->bs) { bdrv_detach_dev(blkdev->bs, blkdev); -bdrv_unref(blkdev->bs); if (!blkdev->dinfo) { blk_unref(blk_by_name(blkdev->dev)); } diff --git a/qemu-img.c b/qemu-img.c index 5548637..09e7e72 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -329,7 +329,6 @@ static BlockBackend *img_open(const char *id, const char *filename, } return blk; fail: -bdrv_unref(bs); blk_unref(blk); return NULL; } @@ -712,9 +711,7 @@ static int img_check(int argc, char **argv) fail: qapi_free_ImageCheck(check); -bdrv_unref(bs); blk_unref(blk); - return ret; } @@ -786,7 +783,6 @@ static int img_commit(int argc, char **argv) break; } -bdrv_unref(bs); blk_unref(blk); if (ret) { return 1; @@ -119
[Qemu-devel] [PATCH v5 21/23] blockdev: Convert qmp_eject(), qmp_change_blockdev() to BlockBackend
Much more command code needs conversion. I'm converting these now because they're using bdrv_dev_* functions, which I'm about to lift into BlockBackend. Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- blockdev.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/blockdev.c b/blockdev.c index 7bbde75..4389575 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1528,8 +1528,10 @@ exit: } -static void eject_device(BlockDriverState *bs, int force, Error **errp) +static void eject_device(BlockBackend *blk, int force, Error **errp) { +BlockDriverState *bs = blk_bs(blk); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { return; } @@ -1553,15 +1555,15 @@ static void eject_device(BlockDriverState *bs, int force, Error **errp) void qmp_eject(const char *device, bool has_force, bool force, Error **errp) { -BlockDriverState *bs; +BlockBackend *blk; -bs = bdrv_find(device); -if (!bs) { +blk = blk_by_name(device); +if (!blk) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } -eject_device(bs, force, errp); +eject_device(blk, force, errp); } void qmp_block_passwd(bool has_device, const char *device, @@ -1620,16 +1622,18 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename, void qmp_change_blockdev(const char *device, const char *filename, const char *format, Error **errp) { +BlockBackend *blk; BlockDriverState *bs; BlockDriver *drv = NULL; int bdrv_flags; Error *err = NULL; -bs = bdrv_find(device); -if (!bs) { +blk = blk_by_name(device); +if (!blk) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } +bs = blk_bs(blk); if (format) { drv = bdrv_find_whitelisted_format(format, bs->read_only); @@ -1639,7 +1643,7 @@ void qmp_change_blockdev(const char *device, const char *filename, } } -eject_device(bs, 0, &err); +eject_device(blk, 0, &err); if (err) { error_propagate(errp, err); return; -- 1.9.3
[Qemu-devel] [PATCH v5 17/23] pc87312: Drop unused members of PC87312State
Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- include/hw/isa/pc87312.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h index befc8bd..bf74470 100644 --- a/include/hw/isa/pc87312.h +++ b/include/hw/isa/pc87312.h @@ -47,13 +47,10 @@ typedef struct PC87312State { struct { ISADevice *dev; -BlockDriverState *drive[2]; -uint32_t base; } fdc; struct { ISADevice *dev; -uint32_t base; } ide; MemoryRegion io; -- 1.9.3
[Qemu-devel] [PATCH v5 08/23] block: Eliminate BlockDriverState member device_name[]
device_name[] can become non-empty only in bdrv_new_root() and bdrv_move_feature_fields(). The latter is used only to undo damage done by bdrv_swap(). The former is called only by blk_new_with_bs(). Therefore, when a BlockDriverState's device_name[] is non-empty, then it's been created with a BlockBackend, and vice versa. Furthermore, blk_new_with_bs() keeps the two names equal. Therefore, device_name[] is redundant. Eliminate it. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block-migration.c | 12 +++- block.c | 49 ++- block/mirror.c| 3 ++- block/qapi.c | 6 +++--- block/qcow.c | 4 ++-- block/qcow2.c | 4 ++-- block/qed.c | 2 +- block/quorum.c| 4 ++-- block/vdi.c | 2 +- block/vhdx.c | 2 +- block/vmdk.c | 4 ++-- block/vpc.c | 2 +- block/vvfat.c | 2 +- blockjob.c| 3 ++- include/block/block.h | 2 +- include/block/block_int.h | 2 -- 16 files changed, 55 insertions(+), 48 deletions(-) diff --git a/block-migration.c b/block-migration.c index cb3e16c..da30e93 100644 --- a/block-migration.c +++ b/block-migration.c @@ -14,7 +14,9 @@ */ #include "qemu-common.h" -#include "block/block_int.h" +#include "block/block.h" +#include "qemu/error-report.h" +#include "qemu/main-loop.h" #include "hw/hw.h" #include "qemu/queue.h" #include "qemu/timer.h" @@ -130,9 +132,9 @@ static void blk_send(QEMUFile *f, BlkMigBlock * blk) | flags); /* device name */ -len = strlen(blk->bmds->bs->device_name); +len = strlen(bdrv_get_device_name(blk->bmds->bs)); qemu_put_byte(f, len); -qemu_put_buffer(f, (uint8_t *)blk->bmds->bs->device_name, len); +qemu_put_buffer(f, (uint8_t *)bdrv_get_device_name(blk->bmds->bs), len); /* if a block is zero we need to flush here since the network * bandwidth is now a lot higher than the storage device bandwidth. @@ -382,9 +384,9 @@ static void init_blk_migration(QEMUFile *f) if (bmds->shared_base) { DPRINTF("Start migration for %s with shared base image\n", -bs->device_name); +bdrv_get_device_name(bs)); } else { -DPRINTF("Start full migration for %s\n", bs->device_name); +DPRINTF("Start full migration for %s\n", bdrv_get_device_name(bs)); } QSIMPLEQ_INSERT_TAIL(&block_mig_state.bmds_list, bmds, entry); diff --git a/block.c b/block.c index 5fd173a..fd57cc0 100644 --- a/block.c +++ b/block.c @@ -28,6 +28,7 @@ #include "block/blockjob.h" #include "qemu/module.h" #include "qapi/qmp/qjson.h" +#include "sysemu/block-backend.h" #include "sysemu/sysemu.h" #include "qemu/notify.h" #include "block/coroutine.h" @@ -360,9 +361,7 @@ BlockDriverState *bdrv_new_root(const char *device_name, Error **errp) bs = bdrv_new(); -pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); - return bs; } @@ -1168,7 +1167,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) } else if (backing_hd) { error_setg(&bs->backing_blocker, "device is used as backing hd of '%s'", - bs->device_name); + bdrv_get_device_name(bs)); } bs->backing_hd = backing_hd; @@ -1542,7 +1541,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } else { error_setg(errp, "Block format '%s' used by device '%s' doesn't " "support the option '%s'", drv->format_name, - bs->device_name, entry->key); + bdrv_get_device_name(bs), entry->key); } ret = -EINVAL; @@ -1749,7 +1748,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) && reopen_state->flags & BDRV_O_RDWR) { error_set(errp, QERR_DEVICE_IS_READ_ONLY, - reopen_state->bs->device_name); + bdrv_get_device_name(reopen_state->bs)); goto error; } @@ -1776,7 +1775,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, /* It is currently mandatory to have a bdrv_reopen_prepare() * handler for each supported drv. */ error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, - drv->format_name, reopen_state->bs->device_name, + drv->format_name, bdrv_get_device_name(reopen_state->bs), "reopening of file"); ret = -1; goto error; @@ -1964,10 +1963,17 @@ void bdrv_drain_all(void) Also, NULL terminate the device_name to prevent double remove */ void b
[Qemu-devel] [PATCH v5 20/23] block/qapi: Convert qmp_query_block() to BlockBackend
Much more command code needs conversion. I start with this one because it's using bdrv_dev_* functions, which I'm about to lift into BlockBackend. While there, give bdrv_query_info() internal linkage. Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- block/qapi.c | 15 --- include/block/qapi.h | 3 --- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index d071ee5..fca981d 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -28,6 +28,7 @@ #include "qapi-visit.h" #include "qapi/qmp-output-visitor.h" #include "qapi/qmp/types.h" +#include "sysemu/block-backend.h" #ifdef __linux__ #include #include @@ -264,15 +265,15 @@ void bdrv_query_image_info(BlockDriverState *bs, } /* @p_info will be set only on success. */ -void bdrv_query_info(BlockDriverState *bs, - BlockInfo **p_info, - Error **errp) +static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, +Error **errp) { BlockInfo *info = g_malloc0(sizeof(*info)); +BlockDriverState *bs = blk_bs(blk); BlockDriverState *bs0; ImageInfo **p_image_info; Error *local_err = NULL; -info->device = g_strdup(bdrv_get_device_name(bs)); +info->device = g_strdup(blk_name(blk)); info->type = g_strdup("unknown"); info->locked = bdrv_dev_is_medium_locked(bs); info->removable = bdrv_dev_has_removable_media(bs); @@ -360,12 +361,12 @@ static BlockStats *bdrv_query_stats(const BlockDriverState *bs) BlockInfoList *qmp_query_block(Error **errp) { BlockInfoList *head = NULL, **p_next = &head; -BlockDriverState *bs = NULL; +BlockBackend *blk; Error *local_err = NULL; - while ((bs = bdrv_next(bs))) { +for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { BlockInfoList *info = g_malloc0(sizeof(*info)); -bdrv_query_info(bs, &info->value, &local_err); +bdrv_query_info(blk, &info->value, &local_err); if (local_err) { error_propagate(errp, local_err); goto err; diff --git a/include/block/qapi.h b/include/block/qapi.h index 0374546..168d788 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -36,9 +36,6 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs, void bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, Error **errp); -void bdrv_query_info(BlockDriverState *bs, - BlockInfo **p_info, - Error **errp); void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f, QEMUSnapshotInfo *sn); -- 1.9.3
[Qemu-devel] [PATCH v5 23/23] block: Make device model's references to BlockBackend strong
Doesn't make a difference just yet, but it's the right thing to do. Signed-off-by: Markus Armbruster Reviewed-by: Benoît Canet Reviewed-by: Kevin Wolf Reviewed-by: Max Reitz --- block/block-backend.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index bdcbac6..d0692b1 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -257,6 +257,7 @@ int blk_attach_dev(BlockBackend *blk, void *dev) if (blk->dev) { return -EBUSY; } +blk_ref(blk); blk->dev = dev; bdrv_iostatus_reset(blk->bs); @@ -290,6 +291,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev) blk->dev_opaque = NULL; bdrv_set_guest_block_size(blk->bs, 512); qemu_coroutine_adjust_pool_size(-COROUTINE_POOL_RESERVATION); +blk_unref(blk); } /* -- 1.9.3
[Qemu-devel] [PATCH v5 11/23] block: Rename BlockDriverAIOCB* to BlockAIOCB*
I'll use BlockDriverAIOCB with block backends shortly, and the name is going to fit badly there. It's a block layer thing anyway, not just a block driver thing. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block-migration.c | 2 +- block.c | 124 ++-- block/archipelago.c | 22 block/blkdebug.c| 10 ++-- block/blkverify.c | 12 ++--- block/curl.c| 4 +- block/iscsi.c | 6 +-- block/linux-aio.c | 6 +-- block/null.c| 34 ++-- block/qed.c | 32 ++-- block/qed.h | 2 +- block/quorum.c | 36 ++--- block/raw-aio.h | 4 +- block/raw-posix.c | 16 +++--- block/raw-win32.c | 8 +-- block/raw_bsd.c | 8 +-- block/rbd.c | 56 ++-- block/sheepdog.c| 4 +- block/win32-aio.c | 4 +- dma-helpers.c | 20 +++ docs/blkdebug.txt | 8 +-- hw/block/nvme.h | 2 +- hw/ide/ahci.h | 2 +- hw/ide/core.c | 8 +-- hw/ide/internal.h | 6 +-- hw/ppc/mac.h| 2 +- include/block/aio.h | 8 +-- include/block/block.h | 34 ++-- include/block/block_int.h | 10 ++-- include/block/thread-pool.h | 2 +- include/hw/scsi/scsi.h | 2 +- include/sysemu/dma.h| 26 +- tests/test-thread-pool.c| 2 +- thread-pool.c | 8 +-- 34 files changed, 265 insertions(+), 265 deletions(-) diff --git a/block-migration.c b/block-migration.c index da30e93..08db01a 100644 --- a/block-migration.c +++ b/block-migration.c @@ -72,7 +72,7 @@ typedef struct BlkMigBlock { int nr_sectors; struct iovec iov; QEMUIOVector qiov; -BlockDriverAIOCB *aiocb; +BlockAIOCB *aiocb; /* Protected by block migration lock. */ int ret; diff --git a/block.c b/block.c index b075669..d2218d1 100644 --- a/block.c +++ b/block.c @@ -61,10 +61,10 @@ struct BdrvDirtyBitmap { #define COROUTINE_POOL_RESERVATION 64 /* number of coroutines to reserve */ static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load); -static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs, +static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, BlockDriverCompletionFunc *cb, void *opaque); -static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState *bs, +static BlockAIOCB *bdrv_aio_writev_em(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, BlockDriverCompletionFunc *cb, void *opaque); static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs, @@ -79,14 +79,14 @@ static int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs, static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); -static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs, - int64_t sector_num, - QEMUIOVector *qiov, - int nb_sectors, - BdrvRequestFlags flags, - BlockDriverCompletionFunc *cb, - void *opaque, - bool is_write); +static BlockAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs, + int64_t sector_num, + QEMUIOVector *qiov, + int nb_sectors, + BdrvRequestFlags flags, + BlockDriverCompletionFunc *cb, + void *opaque, + bool is_write); static void coroutine_fn bdrv_co_do_rw(void *opaque); static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags); @@ -4413,9 +4413,9 @@ int bdrv_get_backing_file_depth(BlockDriverState *bs) /**/ /* async I/Os */ -BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num, - QEMUIOVector *qiov, int nb_sectors, - BlockDriverCompletionFunc *cb, void *opaque) +BlockAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num, + QEMUIOVector *qiov, int nb_sectors, + BlockDriverCompletionFunc *cb, void *opaque) {
[Qemu-devel] [PATCH v5 16/23] ide: Complete conversion from BlockDriverState to BlockBackend
Add a BlockBackend member to TrimAIOCB, so ide_issue_trim_cb() can use blk_aio_discard() instead of bdrv_aio_discard(). Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- hw/ide/core.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 5d3d988..2e8b7d6 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -362,6 +362,7 @@ static void ide_set_signature(IDEState *s) typedef struct TrimAIOCB { BlockAIOCB common; +BlockBackend *blk; QEMUBH *bh; int ret; QEMUIOVector *qiov; @@ -421,8 +422,8 @@ static void ide_issue_trim_cb(void *opaque, int ret) } /* Got an entry! Submit and exit. */ -iocb->aiocb = bdrv_aio_discard(iocb->common.bs, sector, count, - ide_issue_trim_cb, opaque); +iocb->aiocb = blk_aio_discard(iocb->blk, sector, count, + ide_issue_trim_cb, opaque); return; } @@ -446,6 +447,7 @@ BlockAIOCB *ide_issue_trim(BlockBackend *blk, TrimAIOCB *iocb; iocb = blk_aio_get(&trim_aiocb_info, blk, cb, opaque); +iocb->blk = blk; iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb); iocb->ret = 0; iocb->qiov = qiov; -- 1.9.3
[Qemu-devel] [PATCH v5 22/23] block: Lift device model API into BlockBackend
Move device model attachment / detachment and the BlockDevOps device model callbacks and their wrappers from BlockDriverState to BlockBackend. Wrapper calls in block.c change from bdrv_dev_FOO_cb(bs, ...) to if (bs->blk) { bdrv_dev_FOO_cb(bs->blk, ...); } No change, because both bdrv_dev_change_media_cb() and bdrv_dev_resize_cb() do nothing when no device model is attached, and a device model can be attached only when bs->blk. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf --- block.c| 126 -- block/block-backend.c | 151 ++--- block/qapi.c | 8 +-- blockdev.c | 8 +-- include/block/block.h | 45 include/block/block_int.h | 12 ++-- include/sysemu/block-backend.h | 35 ++ 7 files changed, 203 insertions(+), 182 deletions(-) diff --git a/block.c b/block.c index d8d7214..940ecda 100644 --- a/block.c +++ b/block.c @@ -58,9 +58,6 @@ struct BdrvDirtyBitmap { #define NOT_DONE 0x7fff /* used while emulated sync operation in progress */ -#define COROUTINE_POOL_RESERVATION 64 /* number of coroutines to reserve */ - -static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load); static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, BlockCompletionFunc *cb, void *opaque); @@ -1527,7 +1524,9 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } if (!bdrv_key_required(bs)) { -bdrv_dev_change_media_cb(bs, true); +if (bs->blk) { +blk_dev_change_media_cb(bs->blk, true); +} } else if (!runstate_check(RUN_STATE_PRELAUNCH) && !runstate_check(RUN_STATE_INMIGRATE) && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */ @@ -1852,7 +1851,9 @@ void bdrv_close(BlockDriverState *bs) } } -bdrv_dev_change_media_cb(bs, false); +if (bs->blk) { +blk_dev_change_media_cb(bs->blk, false); +} /*throttling disk I/O limits*/ if (bs->io_limits_enabled) { @@ -1971,9 +1972,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, /* move some fields that need to stay attached to the device */ /* dev info */ -bs_dest->dev_ops= bs_src->dev_ops; -bs_dest->dev_opaque = bs_src->dev_opaque; -bs_dest->dev= bs_src->dev; bs_dest->guest_block_size = bs_src->guest_block_size; bs_dest->copy_on_read = bs_src->copy_on_read; @@ -2043,7 +2041,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) assert(!bs_new->blk); assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(bs_new->job == NULL); -assert(bs_new->dev == NULL); assert(bs_new->io_limits_enabled == false); assert(!throttle_have_timer(&bs_new->throttle_state)); @@ -2060,7 +2057,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) assert(!bs_new->blk); /* Check a few fields that should remain attached to the device */ -assert(bs_new->dev == NULL); assert(bs_new->job == NULL); assert(bs_new->io_limits_enabled == false); assert(!throttle_have_timer(&bs_new->throttle_state)); @@ -2099,7 +2095,6 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) static void bdrv_delete(BlockDriverState *bs) { -assert(!bs->dev); assert(!bs->job); assert(bdrv_op_blocker_is_empty(bs)); assert(!bs->refcnt); @@ -2113,105 +2108,6 @@ static void bdrv_delete(BlockDriverState *bs) g_free(bs); } -int bdrv_attach_dev(BlockDriverState *bs, void *dev) -/* TODO change to DeviceState *dev when all users are qdevified */ -{ -if (bs->dev) { -return -EBUSY; -} -bs->dev = dev; -bdrv_iostatus_reset(bs); - -/* We're expecting I/O from the device so bump up coroutine pool size */ -qemu_coroutine_adjust_pool_size(COROUTINE_POOL_RESERVATION); -return 0; -} - -/* TODO qdevified devices don't use this, remove when devices are qdevified */ -void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev) -{ -if (bdrv_attach_dev(bs, dev) < 0) { -abort(); -} -} - -void bdrv_detach_dev(BlockDriverState *bs, void *dev) -/* TODO change to DeviceState *dev when all users are qdevified */ -{ -assert(bs->dev == dev); -bs->dev = NULL; -bs->dev_ops = NULL; -bs->dev_opaque = NULL; -bs->guest_block_size = 512; -qemu_coroutine_adjust_pool_size(-COROUTINE_POOL_RESERVATION); -} - -/* TODO change to return DeviceState * when all users are qdevified */ -void *bdrv_get_attached_dev(BlockDriverState *bs) -{ -return bs->dev; -} - -void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops, - void *opaque) -{ -bs->dev_ops = ops; -bs->dev_opaque = op
[Qemu-devel] [PATCH v5 12/23] block: Rename BlockDriverCompletionFunc to BlockCompletionFunc
I'll use it with block backends shortly, and the name is going to fit badly there. It's a block layer thing anyway, not just a block driver thing. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block.c | 30 +++--- block/archipelago.c | 8 block/backup.c | 2 +- block/blkdebug.c| 8 block/blkverify.c | 8 block/commit.c | 2 +- block/curl.c| 2 +- block/iscsi.c | 2 +- block/linux-aio.c | 2 +- block/mirror.c | 6 +++--- block/null.c| 8 block/qed-gencb.c | 4 ++-- block/qed-table.c | 10 +- block/qed.c | 18 +- block/qed.h | 10 +- block/quorum.c | 6 +++--- block/raw-aio.h | 4 ++-- block/raw-posix.c | 16 block/raw-win32.c | 8 block/raw_bsd.c | 2 +- block/rbd.c | 10 +- block/stream.c | 2 +- block/win32-aio.c | 2 +- blockjob.c | 4 ++-- dma-helpers.c | 2 +- docs/blkdebug.txt | 2 +- hw/ide/ahci.c | 2 +- hw/ide/core.c | 4 ++-- hw/ide/internal.h | 6 +++--- hw/ide/macio.c | 2 +- hw/ide/pci.c| 2 +- hw/ide/pci.h| 2 +- hw/scsi/scsi-generic.c | 2 +- include/block/aio.h | 6 +++--- include/block/block.h | 14 +++--- include/block/block_int.h | 20 ++-- include/block/blockjob.h| 4 ++-- include/block/thread-pool.h | 2 +- include/monitor/monitor.h | 4 ++-- include/sysemu/dma.h| 8 monitor.c | 6 +++--- thread-pool.c | 2 +- 42 files changed, 132 insertions(+), 132 deletions(-) diff --git a/block.c b/block.c index d2218d1..d8d7214 100644 --- a/block.c +++ b/block.c @@ -63,10 +63,10 @@ struct BdrvDirtyBitmap { static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load); static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, -BlockDriverCompletionFunc *cb, void *opaque); +BlockCompletionFunc *cb, void *opaque); static BlockAIOCB *bdrv_aio_writev_em(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, -BlockDriverCompletionFunc *cb, void *opaque); +BlockCompletionFunc *cb, void *opaque); static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *iov); @@ -84,7 +84,7 @@ static BlockAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs, QEMUIOVector *qiov, int nb_sectors, BdrvRequestFlags flags, - BlockDriverCompletionFunc *cb, + BlockCompletionFunc *cb, void *opaque, bool is_write); static void coroutine_fn bdrv_co_do_rw(void *opaque); @@ -4415,7 +4415,7 @@ int bdrv_get_backing_file_depth(BlockDriverState *bs) BlockAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, - BlockDriverCompletionFunc *cb, void *opaque) + BlockCompletionFunc *cb, void *opaque) { trace_bdrv_aio_readv(bs, sector_num, nb_sectors, opaque); @@ -4425,7 +4425,7 @@ BlockAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t sector_num, BlockAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, -BlockDriverCompletionFunc *cb, void *opaque) +BlockCompletionFunc *cb, void *opaque) { trace_bdrv_aio_writev(bs, sector_num, nb_sectors, opaque); @@ -4435,7 +4435,7 @@ BlockAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num, BlockAIOCB *bdrv_aio_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags, -BlockDriverCompletionFunc *cb, void *opaque) +BlockCompletionFunc *cb, void *opaque) { trace_bdrv_aio_write_zeroes(bs, sector_num, nb_sectors, flags, opaque); @@ -4450,7 +4450,7 @@ typedef struct MultiwriteCB { int num_requests; int num_callbacks; struct { -BlockDriverCompletionFunc *cb; +BlockCompletionFunc *cb; void *opaque; QEMUIOVector *free_qiov; } callbacks[]; @@ -4688,7 +4688,7 @@ s
[Qemu-devel] [PATCH v5 19/23] blockdev: Fix blockdev-add not to create DriveInfo
blockdev_init() always creates a DriveInfo, but only drive_new() fills it in. qmp_blockdev_add() leaves it blank. This results in a drive with type = IF_IDE, bus = 0, unit = 0. Screwed up in commit ee13ed1c. Board initialization code looking for IDE drive (0,0) can pick up one of these bogus drives. The QMP command has to execute really early to be visible. Not sure how likely that is in practice. Fix by creating DriveInfo in drive_new(). Block backends created by blockdev-add don't get one. Breaks the test for "has been created by qmp_blockdev_add()" in blockdev_mark_auto_del() and do_drive_del(), because it changes the value of dinfo && !dinfo->enable_auto_del from true to false. Simply test !dinfo instead. Leaves DriveInfo member enable_auto_del unused. Drop it. A few places assume a block backend always has a DriveInfo. Fix them up. Signed-off-by: Markus Armbruster Reviewed-by: Max Reitz --- block/block-backend.c | 2 +- blockdev.c| 22 +++--- hw/block/block.c | 16 ++-- hw/ide/qdev.c | 2 +- hw/scsi/scsi-disk.c | 2 +- include/sysemu/blockdev.h | 1 - 6 files changed, 20 insertions(+), 25 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index a3e613b..ae30873 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -18,7 +18,7 @@ struct BlockBackend { char *name; int refcnt; BlockDriverState *bs; -DriveInfo *legacy_dinfo; +DriveInfo *legacy_dinfo;/* null unless created by drive_new() */ QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */ }; diff --git a/blockdev.c b/blockdev.c index b102fe0..7bbde75 100644 --- a/blockdev.c +++ b/blockdev.c @@ -90,16 +90,14 @@ void blockdev_mark_auto_del(BlockBackend *blk) DriveInfo *dinfo = blk_legacy_dinfo(blk); BlockDriverState *bs = blk_bs(blk); -if (dinfo && !dinfo->enable_auto_del) { +if (!dinfo) { return; } if (bs->job) { block_job_cancel(bs->job); } -if (dinfo) { -dinfo->auto_del = 1; -} +dinfo->auto_del = 1; } void blockdev_auto_del(BlockBackend *blk) @@ -285,7 +283,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, int on_read_error, on_write_error; BlockBackend *blk; BlockDriverState *bs; -DriveInfo *dinfo; ThrottleConfig cfg; int snapshot = 0; bool copy_on_read; @@ -457,9 +454,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, bdrv_set_io_limits(bs, &cfg); } -dinfo = g_malloc0(sizeof(*dinfo)); -blk_set_legacy_dinfo(blk, dinfo); - if (!file || !*file) { if (has_driver_specific_opts) { file = NULL; @@ -925,9 +919,8 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) assert(!local_err); } -/* Set legacy DriveInfo fields */ -dinfo = blk_legacy_dinfo(blk); -dinfo->enable_auto_del = true; +/* Create legacy DriveInfo */ +dinfo = g_malloc0(sizeof(*dinfo)); dinfo->opts = all_opts; dinfo->cyls = cyls; @@ -939,9 +932,10 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->bus = bus_id; dinfo->unit = unit_id; dinfo->devaddr = devaddr; - dinfo->serial = g_strdup(serial); +blk_set_legacy_dinfo(blk, dinfo); + switch(type) { case IF_IDE: case IF_SCSI: @@ -1745,7 +1739,6 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) const char *id = qdict_get_str(qdict, "id"); BlockBackend *blk; BlockDriverState *bs; -DriveInfo *dinfo; AioContext *aio_context; Error *local_err = NULL; @@ -1756,8 +1749,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) } bs = blk_bs(blk); -dinfo = blk_legacy_dinfo(blk); -if (dinfo && !dinfo->enable_auto_del) { +if (!blk_legacy_dinfo(blk)) { error_report("Deleting device added with blockdev-add" " is not supported"); return -1; diff --git a/hw/block/block.c b/hw/block/block.c index 0666dd3..a625773 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -19,7 +19,9 @@ void blkconf_serial(BlockConf *conf, char **serial) if (!*serial) { /* try to fall back to value set with legacy -drive serial=... */ dinfo = blk_legacy_dinfo(conf->blk); -*serial = g_strdup(dinfo->serial); +if (dinfo) { +*serial = g_strdup(dinfo->serial); +} } } @@ -32,11 +34,13 @@ void blkconf_geometry(BlockConf *conf, int *ptrans, if (!conf->cyls && !conf->heads && !conf->secs) { /* try to fall back to value set with legacy -drive cyls=... */ dinfo = blk_legacy_dinfo(conf->blk); -conf->cyls = dinfo->cyls; -conf->heads = dinfo->heads; -conf->secs = dinfo->secs; -if (ptrans) { -*ptran
Re: [Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate for incoming migration
Il 02/10/2014 10:52, Alexey Kardashevskiy ha scritto: > When migrated using libvirt with "--copy-storage-all", at the end of > migration there is race between NBD mirroring task trying to do flush > and migration completion, both end up invalidating cache. Since qcow2 > driver does not handle this situation very well, random crashes happen. > > This disables the BDRV_O_INCOMING flag for the block device being migrated > and restores it when NBD task is done. > > Signed-off-by: Alexey Kardashevskiy > --- > > > The commit log is not full and most likely incorrect as well > as the patch :) Please, help. Thanks! > > The patch seems to fix the initial problem though. > > > btw is there any easy way to migrate one QEMU to another > using NBD (i.e. not using "migrate -b") and not using libvirt? > What would the command line be? Debugging with libvirt is real > pain :( > > > --- > block.c | 17 - > migration.c | 1 - > nbd.c | 11 +++ > 3 files changed, 15 insertions(+), 14 deletions(-) > > diff --git a/block.c b/block.c > index c5a251c..ed72e0a 100644 > --- a/block.c > +++ b/block.c > @@ -5073,6 +5073,10 @@ void bdrv_invalidate_cache_all(Error **errp) > QTAILQ_FOREACH(bs, &bdrv_states, device_list) { > AioContext *aio_context = bdrv_get_aio_context(bs); > > +if (!(bs->open_flags & BDRV_O_INCOMING)) { > +continue; > +} > + > aio_context_acquire(aio_context); > bdrv_invalidate_cache(bs, &local_err); > aio_context_release(aio_context); This part is okay, though perhaps we should add it to bdrv_invalidate_cache instead? > @@ -5083,19 +5087,6 @@ void bdrv_invalidate_cache_all(Error **errp) > } > } > > -void bdrv_clear_incoming_migration_all(void) > -{ > -BlockDriverState *bs; > - > -QTAILQ_FOREACH(bs, &bdrv_states, device_list) { > -AioContext *aio_context = bdrv_get_aio_context(bs); > - > -aio_context_acquire(aio_context); > -bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING); > -aio_context_release(aio_context); > -} > -} > - > int bdrv_flush(BlockDriverState *bs) > { > Coroutine *co; > diff --git a/migration.c b/migration.c > index 8d675b3..c49a05a 100644 > --- a/migration.c > +++ b/migration.c > @@ -103,7 +103,6 @@ static void process_incoming_migration_co(void *opaque) > } > qemu_announce_self(); > > -bdrv_clear_incoming_migration_all(); > /* Make sure all file formats flush their mutable metadata */ > bdrv_invalidate_cache_all(&local_err); > if (local_err) { This part I don't understand. Shouldn't you at least be adding bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING); to bdrv_invalidate_cache? > diff --git a/nbd.c b/nbd.c > index e9b539b..7b479c0 100644 > --- a/nbd.c > +++ b/nbd.c > @@ -106,6 +106,7 @@ struct NBDExport { > off_t dev_offset; > off_t size; > uint32_t nbdflags; > +bool restore_incoming; > QTAILQ_HEAD(, NBDClient) clients; > QTAILQ_ENTRY(NBDExport) next; > > @@ -972,6 +973,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t > dev_offset, > exp->ctx = bdrv_get_aio_context(bs); > bdrv_ref(bs); > bdrv_add_aio_context_notifier(bs, bs_aio_attached, bs_aio_detach, exp); > + > +if (bs->open_flags & BDRV_O_INCOMING) { > +bdrv_invalidate_cache(bs, NULL); > +exp->restore_incoming = !!(bs->open_flags & BDRV_O_INCOMING); > +bs->open_flags &= ~(BDRV_O_INCOMING); > +} > + > return exp; > } > > @@ -1021,6 +1029,9 @@ void nbd_export_close(NBDExport *exp) > if (exp->bs) { > bdrv_remove_aio_context_notifier(exp->bs, bs_aio_attached, > bs_aio_detach, exp); > +if (exp->restore_incoming) { > +exp->bs->open_flags |= BDRV_O_INCOMING; > +} > bdrv_unref(exp->bs); > exp->bs = NULL; > } > For this, I don't think you even need exp->restore_incoming, and then it can simply be a one-liner + bdrv_invalidate_cache(bs, NULL); if you modify bdrv_invalidate_cache instead of bdrv_invalidate_cache_all. Paolo
[Qemu-devel] [PATCH 0/2] Here are 2 patches to enable sPAPR NVRAM migration.
Please comment. Thanks! Changes: v2: * reworked to always have a copy of NVRAM in RAM and use file only for writes Alexey Kardashevskiy (2): vmstate: Allow dynamic allocation for VBUFFER during migration spapr_nvram: Enable migration hw/nvram/spapr_nvram.c | 81 - include/migration/vmstate.h | 11 ++ vmstate.c | 13 ++-- 3 files changed, 86 insertions(+), 19 deletions(-) -- 2.0.0
[Qemu-devel] [PATCH 2/2] spapr_nvram: Enable migration
The only case when sPAPR NVRAM migrates now is if is backed by a file and copy-storage migration is performed. In other cases NVRAM does not migrate regardless whether it is backed by a file or not. This enables shadow copy of NVRAM in RAM which is read from a file (if used) and used for reads. Writes to NVRAM are mirrored to the file. This defines a VMSTATE descriptor for NVRAM device so the memory copy of NVRAM can migrate and be flushed to a backing file on the destination if one is specified. Signed-off-by: Alexey Kardashevskiy --- Changes: v2: * implemented full-time shadow copy of NVRAM which is read from a file at start and stays in RAM during live of the guest --- hw/nvram/spapr_nvram.c | 81 -- 1 file changed, 65 insertions(+), 16 deletions(-) diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c index 6a72ef4..d34edfb 100644 --- a/hw/nvram/spapr_nvram.c +++ b/hw/nvram/spapr_nvram.c @@ -51,7 +51,6 @@ static void rtas_nvram_fetch(PowerPCCPU *cpu, sPAPREnvironment *spapr, { sPAPRNVRAM *nvram = spapr->nvram; hwaddr offset, buffer, len; -int alen; void *membuf; if ((nargs != 3) || (nret != 2)) { @@ -76,19 +75,16 @@ static void rtas_nvram_fetch(PowerPCCPU *cpu, sPAPREnvironment *spapr, return; } +assert(nvram->buf); + membuf = cpu_physical_memory_map(buffer, &len, 1); -if (nvram->drive) { -alen = bdrv_pread(nvram->drive, offset, membuf, len); -} else { -assert(nvram->buf); -memcpy(membuf, nvram->buf + offset, len); -alen = len; -} +memcpy(membuf, nvram->buf + offset, len); + cpu_physical_memory_unmap(membuf, len, 1, len); -rtas_st(rets, 0, (alen < len) ? RTAS_OUT_HW_ERROR : RTAS_OUT_SUCCESS); -rtas_st(rets, 1, (alen < 0) ? 0 : alen); +rtas_st(rets, 0, RTAS_OUT_SUCCESS); +rtas_st(rets, 1, len); } static void rtas_nvram_store(PowerPCCPU *cpu, sPAPREnvironment *spapr, @@ -122,14 +118,15 @@ static void rtas_nvram_store(PowerPCCPU *cpu, sPAPREnvironment *spapr, } membuf = cpu_physical_memory_map(buffer, &len, 0); + +alen = len; if (nvram->drive) { alen = bdrv_pwrite(nvram->drive, offset, membuf, len); -} else { -assert(nvram->buf); - -memcpy(nvram->buf + offset, membuf, len); -alen = len; } + +assert(nvram->buf); +memcpy(nvram->buf + offset, membuf, len); + cpu_physical_memory_unmap(membuf, len, 0, len); rtas_st(rets, 0, (alen < len) ? RTAS_OUT_HW_ERROR : RTAS_OUT_SUCCESS); @@ -144,15 +141,24 @@ static int spapr_nvram_init(VIOsPAPRDevice *dev) nvram->size = bdrv_getlength(nvram->drive); } else { nvram->size = DEFAULT_NVRAM_SIZE; -nvram->buf = g_malloc0(nvram->size); } +nvram->buf = g_malloc0(nvram->size); + if ((nvram->size < MIN_NVRAM_SIZE) || (nvram->size > MAX_NVRAM_SIZE)) { fprintf(stderr, "spapr-nvram must be between %d and %d bytes in size\n", MIN_NVRAM_SIZE, MAX_NVRAM_SIZE); return -1; } +if (nvram->drive) { +int alen = bdrv_pread(nvram->drive, 0, nvram->buf, nvram->size); + +if (alen != nvram->size) { +return -1; +} +} + spapr_rtas_register(RTAS_NVRAM_FETCH, "nvram-fetch", rtas_nvram_fetch); spapr_rtas_register(RTAS_NVRAM_STORE, "nvram-store", rtas_nvram_store); @@ -166,6 +172,48 @@ static int spapr_nvram_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off) return fdt_setprop_cell(fdt, node_off, "#bytes", nvram->size); } +static int spapr_nvram_pre_load(void *opaque) +{ +sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque); + +g_free(nvram->buf); +nvram->buf = NULL; +nvram->size = 0; + +return 0; +} + +static int spapr_nvram_post_load(void *opaque, int version_id) +{ +sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque); + +if (nvram->drive) { +int alen = bdrv_pwrite(nvram->drive, 0, nvram->buf, nvram->size); + +if (alen < 0) { +return alen; +} +if (alen != nvram->size) { +return -1; +} +} + +return 0; +} + +static const VMStateDescription vmstate_spapr_nvram = { +.name = "spapr_nvram", +.version_id = 1, +.minimum_version_id = 1, +.pre_load = spapr_nvram_pre_load, +.post_load = spapr_nvram_post_load, +.fields = (VMStateField[]) { +VMSTATE_UINT32(size, sPAPRNVRAM), +VMSTATE_VBUFFER_ALLOC_UINT32(buf, sPAPRNVRAM, 1, NULL, 0, size), +VMSTATE_END_OF_LIST() +}, +}; + static Property spapr_nvram_properties[] = { DEFINE_SPAPR_PROPERTIES(sPAPRNVRAM, sdev), DEFINE_PROP_DRIVE("drive", sPAPRNVRAM, drive), @@ -184,6 +232,7 @@ static void spapr_nvram_class_init(ObjectClass *klass, void *data) k->dt_compatible = "qemu,spapr-nvram"; set_bit(DEVICE_CATEGORY_MISC, dc->categories); dc->props = spapr_nvram_properties; +dc->vmsd = &vm
[Qemu-devel] [PATCH 1/2] vmstate: Allow dynamic allocation for VBUFFER during migration
This extends use of VMS_ALLOC flag from arrays to VBUFFER as well. This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of VMS_ALLOC and uses uint32_t type for a size. Signed-off-by: Alexey Kardashevskiy --- include/migration/vmstate.h | 11 +++ vmstate.c | 13 ++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 9a001bd..e45fc49 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -484,6 +484,17 @@ extern const VMStateInfo vmstate_info_bitmap; .start= (_start),\ } +#define VMSTATE_VBUFFER_ALLOC_UINT32(_field, _state, _version, _test, _start, _field_size) { \ +.name = (stringify(_field)), \ +.version_id = (_version), \ +.field_exists = (_test), \ +.size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\ +.info = &vmstate_info_buffer,\ +.flags= VMS_VBUFFER|VMS_POINTER|VMS_ALLOC, \ +.offset = offsetof(_state, _field),\ +.start= (_start),\ +} + #define VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, _info, _size) { \ .name = (stringify(_field)), \ .version_id = (_version),\ diff --git a/vmstate.c b/vmstate.c index ef2f87b..3dde574 100644 --- a/vmstate.c +++ b/vmstate.c @@ -49,9 +49,16 @@ static void *vmstate_base_addr(void *opaque, VMStateField *field, bool alloc) if (field->flags & VMS_POINTER) { if (alloc && (field->flags & VMS_ALLOC)) { -int n_elems = vmstate_n_elems(opaque, field); -if (n_elems) { -gsize size = n_elems * field->size; +gsize size = 0; +if (field->flags & VMS_VBUFFER) { +size = vmstate_size(opaque, field); +} else { +int n_elems = vmstate_n_elems(opaque, field); +if (n_elems) { +size = n_elems * field->size; +} +} +if (size) { *((void **)base_addr + field->start) = g_malloc(size); } } -- 2.0.0
[Qemu-devel] [PATCH v2 37/36] qdev: device_del: search for to be unplugged device in 'peripheral' container
device_add puts every device with 'id' inside of 'peripheral' container using id's value as the last component name. Use it by replacing recursive search on sysbus with path lookup in 'peripheral' container, which could handle both BUS and BUS-less device cases. Signed-off-by: Igor Mammedov --- qdev-monitor.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index c721451..754437b 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -686,15 +686,20 @@ int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data) void qmp_device_del(const char *id, Error **errp) { -DeviceState *dev; +Object *obj; +char *root_path = object_get_canonical_path(qdev_get_peripheral()); +char *path = g_strdup_printf("%s/%s", root_path, id); -dev = qdev_find_recursive(sysbus_get_default(), id); -if (!dev) { +g_free(root_path); +obj = object_resolve_path_type(path, TYPE_DEVICE, NULL); +g_free(path); + +if (!obj) { error_set(errp, QERR_DEVICE_NOT_FOUND, id); return; } -qdev_unplug(dev, errp); +qdev_unplug(DEVICE(obj), errp); } void qdev_machine_init(void) -- 1.8.3.1
Re: [Qemu-devel] [RFC PATCH] block/migration: Disable cache invalidate for incoming migration
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/02/2014 07:45 PM, Paolo Bonzini wrote: > Il 02/10/2014 10:52, Alexey Kardashevskiy ha scritto: >> When migrated using libvirt with "--copy-storage-all", at the end >> of migration there is race between NBD mirroring task trying to do >> flush and migration completion, both end up invalidating cache. >> Since qcow2 driver does not handle this situation very well, random >> crashes happen. >> >> This disables the BDRV_O_INCOMING flag for the block device being >> migrated and restores it when NBD task is done. >> >> Signed-off-by: Alexey Kardashevskiy --- >> >> >> The commit log is not full and most likely incorrect as well as the >> patch :) Please, help. Thanks! >> >> The patch seems to fix the initial problem though. >> >> >> btw is there any easy way to migrate one QEMU to another using NBD >> (i.e. not using "migrate -b") and not using libvirt? What would the >> command line be? Debugging with libvirt is real pain :( >> >> >> --- block.c | 17 - migration.c | 1 - nbd.c >> | 11 +++ 3 files changed, 15 insertions(+), 14 deletions(-) >> >> diff --git a/block.c b/block.c index c5a251c..ed72e0a 100644 --- >> a/block.c +++ b/block.c @@ -5073,6 +5073,10 @@ void >> bdrv_invalidate_cache_all(Error **errp) QTAILQ_FOREACH(bs, >> &bdrv_states, device_list) { AioContext *aio_context = >> bdrv_get_aio_context(bs); >> >> +if (!(bs->open_flags & BDRV_O_INCOMING)) { + >> continue; +} + aio_context_acquire(aio_context); >> bdrv_invalidate_cache(bs, &local_err); >> aio_context_release(aio_context); > > This part is okay, though perhaps we should add it to > bdrv_invalidate_cache instead? Yes, makes perfect sense. > >> @@ -5083,19 +5087,6 @@ void bdrv_invalidate_cache_all(Error **errp) >> } } >> >> -void bdrv_clear_incoming_migration_all(void) -{ - >> BlockDriverState *bs; - -QTAILQ_FOREACH(bs, &bdrv_states, >> device_list) { -AioContext *aio_context = >> bdrv_get_aio_context(bs); - - >> aio_context_acquire(aio_context); -bs->open_flags = >> bs->open_flags & ~(BDRV_O_INCOMING); - >> aio_context_release(aio_context); -} -} - int >> bdrv_flush(BlockDriverState *bs) { Coroutine *co; diff --git >> a/migration.c b/migration.c index 8d675b3..c49a05a 100644 --- >> a/migration.c +++ b/migration.c @@ -103,7 +103,6 @@ static void >> process_incoming_migration_co(void *opaque) } qemu_announce_self(); >> >> -bdrv_clear_incoming_migration_all(); /* Make sure all file >> formats flush their mutable metadata */ >> bdrv_invalidate_cache_all(&local_err); if (local_err) { > > This part I don't understand. > > Shouldn't you at least be adding > > bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING); > > to bdrv_invalidate_cache? Reset the flag after caches has been invalidated? What is the exact semantic of this BDRV_O_INCOMING? blockdev_init() sets it, we reset it on the first bdrv_invalidate_cache() and then we never set it again? I am still missing the bigger picture... >> diff --git a/nbd.c b/nbd.c index e9b539b..7b479c0 100644 --- >> a/nbd.c +++ b/nbd.c @@ -106,6 +106,7 @@ struct NBDExport { off_t >> dev_offset; off_t size; uint32_t nbdflags; +bool >> restore_incoming; QTAILQ_HEAD(, NBDClient) clients; >> QTAILQ_ENTRY(NBDExport) next; >> >> @@ -972,6 +973,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, >> off_t dev_offset, exp->ctx = bdrv_get_aio_context(bs); >> bdrv_ref(bs); bdrv_add_aio_context_notifier(bs, bs_aio_attached, >> bs_aio_detach, exp); + +if (bs->open_flags & BDRV_O_INCOMING) { >> +bdrv_invalidate_cache(bs, NULL); + >> exp->restore_incoming = !!(bs->open_flags & BDRV_O_INCOMING); + >> bs->open_flags &= ~(BDRV_O_INCOMING); +} + return exp; } >> >> @@ -1021,6 +1029,9 @@ void nbd_export_close(NBDExport *exp) if >> (exp->bs) { bdrv_remove_aio_context_notifier(exp->bs, >> bs_aio_attached, bs_aio_detach, exp); +if >> (exp->restore_incoming) { +exp->bs->open_flags |= >> BDRV_O_INCOMING; +} bdrv_unref(exp->bs); exp->bs = NULL; } >> > > For this, I don't think you even need exp->restore_incoming, and then > it can simply be a one-liner > > + bdrv_invalidate_cache(bs, NULL); > > if you modify bdrv_invalidate_cache instead of > bdrv_invalidate_cache_all. I did not understand that modification but if I do not restore BDRV_O_INCOMING, then changes to the disk I made on the source side before migration - they are lost after rebooting the destination guest. - -- Alexey -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAEBAgAGBQJULSalAAoJEIYTPdgrwSC5tVcP/RAKlDwP2E3hRpfqK4oR+BnR /OF89+kVvlvXtKSImY1/8oHlPwoKIqA974ZuxYnJNZw3xx2xDmMnT3V3UOVs77Te rRhs87ps/xjk+FXrqRQnuITyoJzOCjIuhkx5cVO66caLyfJaesbPmKgPbThH3EoI FHbwe/XsKjttMGAwd721tDrx/1fwAp5BnpFOMP2ZgqMGkRC3+9+xnxIWqOUvpMTl AVsjWvWO5rRSyj/QE+8RQi+XNPtqfiCYaUHLNy+g23GQjIAjol+zY88sS5f9axJD e4BthhumaALrCfJXf/3p0kszV+oUZ6SSnFcbZnMNe90o5+erDjNEt2i2HGW82sPY 42NP6Tpdg3q
Re: [Qemu-devel] [PATCH v3 18/21] target-mips: do not allow Status.FR=0 mode in 64-bit FPU
There is a block of code that modifies CP0_Status_rw_bitmask.CP0St_FR bit to read-writable in the same function. So effectively in case of MIPS64 R6 the bit is now R/W which shouldn't be. You need to modify or merge the code. # if defined(TARGET_MIPS64) /* For MIPS64, init FR bit to 1 if FPU unit is there and bit is writable. */ if ((env->CP0_Config1 & (1 << CP0C1_FP)) && (env->CP0_Status_rw_bitmask & (1 << CP0St_FR))) { env->CP0_Status |= (1 << CP0St_FR); } # endif Regards, Yongbok On 27/06/14 16:22, Leon Alrae wrote: > Status.FR bit must be ignored on write and read as 1 when an implementation of > Release 6 of the Architecture in which a 64-bit floating point unit is > implemented. > > Signed-off-by: Leon Alrae > --- > v3: > * remove line modifying CP0_Status_rw_bitmask as this is done while defining > CPU > --- > target-mips/translate.c |6 ++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index a804322..7cfda3d 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -17942,6 +17942,12 @@ void cpu_state_reset(CPUMIPSState *env) > } > } > #endif > +if ((env->insn_flags & ISA_MIPS32R6) && > +(env->active_fpu.fcr0 & (1 << FCR0_F64))) { > +/* Status.FR = 0 mode in 64-bit FPU not allowed in R6 */ > +env->CP0_Status |= (1 << CP0St_FR); > +} > + > compute_hflags(env); > cs->exception_index = EXCP_NONE; > }
Re: [Qemu-devel] [PATCH v3 18/21] target-mips: do not allow Status.FR=0 mode in 64-bit FPU
Correction: it is not updating the bit but checking the bit. Reviewed-by: Yongbok Kim On 02/10/14 11:21, Yongbok Kim wrote: > There is a block of code that modifies CP0_Status_rw_bitmask.CP0St_FR > bit to read-writable in the same function. > So effectively in case of MIPS64 R6 the bit is now R/W which shouldn't be. > You need to modify or merge the code. > > # if defined(TARGET_MIPS64) > /* For MIPS64, init FR bit to 1 if FPU unit is there and bit is > writable. */ > if ((env->CP0_Config1 & (1 << CP0C1_FP)) && > (env->CP0_Status_rw_bitmask & (1 << CP0St_FR))) { > env->CP0_Status |= (1 << CP0St_FR); > } > # endif > > Regards, > Yongbok > > > On 27/06/14 16:22, Leon Alrae wrote: >> Status.FR bit must be ignored on write and read as 1 when an implementation >> of >> Release 6 of the Architecture in which a 64-bit floating point unit is >> implemented. >> >> Signed-off-by: Leon Alrae >> --- >> v3: >> * remove line modifying CP0_Status_rw_bitmask as this is done while defining >> CPU >> --- >> target-mips/translate.c |6 ++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/target-mips/translate.c b/target-mips/translate.c >> index a804322..7cfda3d 100644 >> --- a/target-mips/translate.c >> +++ b/target-mips/translate.c >> @@ -17942,6 +17942,12 @@ void cpu_state_reset(CPUMIPSState *env) >> } >> } >> #endif >> +if ((env->insn_flags & ISA_MIPS32R6) && >> +(env->active_fpu.fcr0 & (1 << FCR0_F64))) { >> +/* Status.FR = 0 mode in 64-bit FPU not allowed in R6 */ >> +env->CP0_Status |= (1 << CP0St_FR); >> +} >> + >> compute_hflags(env); >> cs->exception_index = EXCP_NONE; >> } >
Re: [Qemu-devel] [PATCH v5 03/23] block: Connect BlockBackend to BlockDriverState
Am 02.10.2014 um 11:04 hat Markus Armbruster geschrieben: > Convenience function blk_new_with_bs() creates a BlockBackend with its > BlockDriverState. Callers have to unref both. The commit after next > will relieve them of the need to unref the BlockDriverState. > > Complication: due to the silly way drive_del works, we need a way to > hide a BlockBackend, just like bdrv_make_anon(). To emphasize its > "special" status, give the function a suitably off-putting name: > blk_hide_on_behalf_of_do_drive_del(). Unfortunately, hiding turns the > BlockBackend's name into the empty string. Can't avoid that without > breaking the blk->bs->device_name equals blk->name invariant. > > The patch adds a memory leak: drive_del while a device model is > connected leaks the BlockBackend. Avoiding the leak here is rather > hairy, but it'll become straightforward shortly, so I mark it FIXME in > the code now, and plug it when it's easy. Does this leak actually still exist now that you have a blk_unref() in drive_del() (which is called during autodel) rather than do_drive_del()? > Signed-off-by: Markus Armbruster > Reviewed-by: Max Reitz Kevin
Re: [Qemu-devel] [PATCH v5 04/23] block: Connect BlockBackend and DriveInfo
Am 02.10.2014 um 11:04 hat Markus Armbruster geschrieben: > Make the BlockBackend own the DriveInfo. Change blockdev_init() to > return the BlockBackend instead of the DriveInfo. > > Signed-off-by: Markus Armbruster > Reviewed-by: Max Reitz > @@ -200,11 +202,11 @@ DriveInfo *drive_get_next(BlockInterfaceType type) > > DriveInfo *drive_get_by_blockdev(BlockDriverState *bs) > { > -DriveInfo *dinfo; > +BlockBackend *blk; > > -QTAILQ_FOREACH(dinfo, &drives, next) { > -if (dinfo->bdrv == bs) { > -return dinfo; > +for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { > +if (blk == bs->blk) { > +return blk_legacy_dinfo(blk); > } > } > return NULL; In v3, I asked why you don't use bs->blk here. Apparently you understood this as a suggestion to change the if condition from: if (blk_bs(blk) == bs) to: if (blk == bs->blk) Which isn't a wrong change, it just doesn't change a lot. What I really meant is something like this, removing the loop: DriveInfo *drive_get_by_blockdev(BlockDriverState *bs) { return bs->blk ? blk_legacy_dinfo(bs->blk) : NULL; } This would only behave differently if there were BlockBackends that can be assigned to bs->blk, but aren't iterated by blk_next(). But such BlockBackends don't exist, blk_next() includes all of them. Kevin
Re: [Qemu-devel] [PATCH 0/2] Here are 2 patches to enable sPAPR NVRAM migration.
On 02.10.14 11:56, Alexey Kardashevskiy wrote: > Please comment. Thanks! > > Changes: > v2: > * reworked to always have a copy of NVRAM in RAM and use file only for writes I like it, please get an ack from Juan on patch 1/2, then I can apply it. Alex
Re: [Qemu-devel] NBD TLS support in QEMU
Il 01/10/2014 22:23, Wouter Verhelst ha scritto: > Hi, > > On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote: >> Tunneling the entire protocol inside an SSL connection doesn't fix that; >> if an attacker is able to hijack your TCP connections and change flags, >> then this attacker is also able to hijack your TCP connection and >> redirect it to a decrypting/encrypting proxy. >> >> I agree that preventing a possible SSL downgrade attack (and other forms >> of MITM) should be high on the priority list, but "tunnel the whole >> thing in SSL" doesn't do that. > > So, having given this some thought, I wanted to come up with a spec just > so that we had something we could all agree on. As part of that, I had a > look at qemu-nbd, and noticed that it uses the "oldstyle" handshake > protocol (on port 10809 by default -- ew, please don't do that). Can you use new-style handshake with a single unnamed export? Export names are a useless complication for qemu-nbd. Paolo
Re: [Qemu-devel] NBD TLS support in QEMU
On Wed, Oct 01, 2014 at 10:23:26PM +0200, Wouter Verhelst wrote: > Hi, > > On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote: > > Tunneling the entire protocol inside an SSL connection doesn't fix that; > > if an attacker is able to hijack your TCP connections and change flags, > > then this attacker is also able to hijack your TCP connection and > > redirect it to a decrypting/encrypting proxy. > > > > I agree that preventing a possible SSL downgrade attack (and other forms > > of MITM) should be high on the priority list, but "tunnel the whole > > thing in SSL" doesn't do that. > > So, having given this some thought, I wanted to come up with a spec just > so that we had something we could all agree on. As part of that, I had a > look at qemu-nbd, and noticed that it uses the "oldstyle" handshake > protocol (on port 10809 by default -- ew, please don't do that). > > I had to change the protocol incompatibly a few years back, because the > oldstyle protocol is broken by design; in the oldstyle negotiation > protocol, the server dumps all information it has on the export to the > client, and then moves on to the data negotiation phase, without waiting > for any reply from the client. This means the oldstyle protocol can't be > used for any sort of negotiation[1]. > > As such, I strongly suggest that qemu-nbd move to the newstyle protocol. Even if we added support for the newstyle protocol I don't see us being able to drop the oldstyle protocol. NBD is used during migration of block storage, and we need to be able to migrate from old QEMU to new QEMU and vica-verca, so can't just switch protocol in a new QEMU without retaining a way to use the old protocol. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [Bug 1376675] [NEW] Qemu UI generally broken (both SDL and Gtk)
Public bug reported: With 2.1.2 on FC20 fedora-virt-preview, both the Gtk Interface and the SDL Interface have bugs. The Gtk Interface has only one: Fullscreen is broken, menu remains visible (w GTK3) The SDL interface is completely broken. -no-frame is not respected. Shortcuts don't work (neither does any variant thereof such as -alt- grab) and the title says "Press Ctrl-Alt to exit mouse-grab". ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1376675 Title: Qemu UI generally broken (both SDL and Gtk) Status in QEMU: New Bug description: With 2.1.2 on FC20 fedora-virt-preview, both the Gtk Interface and the SDL Interface have bugs. The Gtk Interface has only one: Fullscreen is broken, menu remains visible (w GTK3) The SDL interface is completely broken. -no-frame is not respected. Shortcuts don't work (neither does any variant thereof such as -alt- grab) and the title says "Press Ctrl-Alt to exit mouse-grab". To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1376675/+subscriptions
Re: [Qemu-devel] NBD TLS support in QEMU
Il 02/10/2014 13:05, Daniel P. Berrange ha scritto: > On Wed, Oct 01, 2014 at 10:23:26PM +0200, Wouter Verhelst wrote: >> Hi, >> >> On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote: >>> Tunneling the entire protocol inside an SSL connection doesn't fix that; >>> if an attacker is able to hijack your TCP connections and change flags, >>> then this attacker is also able to hijack your TCP connection and >>> redirect it to a decrypting/encrypting proxy. >>> >>> I agree that preventing a possible SSL downgrade attack (and other forms >>> of MITM) should be high on the priority list, but "tunnel the whole >>> thing in SSL" doesn't do that. >> >> So, having given this some thought, I wanted to come up with a spec just >> so that we had something we could all agree on. As part of that, I had a >> look at qemu-nbd, and noticed that it uses the "oldstyle" handshake >> protocol (on port 10809 by default -- ew, please don't do that). >> >> I had to change the protocol incompatibly a few years back, because the >> oldstyle protocol is broken by design; in the oldstyle negotiation >> protocol, the server dumps all information it has on the export to the >> client, and then moves on to the data negotiation phase, without waiting >> for any reply from the client. This means the oldstyle protocol can't be >> used for any sort of negotiation[1]. >> >> As such, I strongly suggest that qemu-nbd move to the newstyle protocol. > > Even if we added support for the newstyle protocol I don't see us being > able to drop the oldstyle protocol. NBD is used during migration of block > storage, and we need to be able to migrate from old QEMU to new QEMU and > vica-verca, so can't just switch protocol in a new QEMU without retaining > a way to use the old protocol. For that you don't use qemu-nbd, we use the NBD server that is embedded in the QEMU executable. That one shares almost all the code with qemu-nbd but, because we are exporting multiple disks over a single port, ends up using the new-style protocol. qemu-nbd uses the old-style protocol only because it has a single, unnamed export. QEMU's NBD client will use the old-style protocol if given URLs like nbd://HOST:PORT/, and the new-style protocol for nbd://HOST:PORT/NAME. Paolo
Re: [Qemu-devel] [PATCH v5 03/23] block: Connect BlockBackend to BlockDriverState
Kevin Wolf writes: > Am 02.10.2014 um 11:04 hat Markus Armbruster geschrieben: >> Convenience function blk_new_with_bs() creates a BlockBackend with its >> BlockDriverState. Callers have to unref both. The commit after next >> will relieve them of the need to unref the BlockDriverState. >> >> Complication: due to the silly way drive_del works, we need a way to >> hide a BlockBackend, just like bdrv_make_anon(). To emphasize its >> "special" status, give the function a suitably off-putting name: >> blk_hide_on_behalf_of_do_drive_del(). Unfortunately, hiding turns the >> BlockBackend's name into the empty string. Can't avoid that without >> breaking the blk->bs->device_name equals blk->name invariant. >> >> The patch adds a memory leak: drive_del while a device model is >> connected leaks the BlockBackend. Avoiding the leak here is rather >> hairy, but it'll become straightforward shortly, so I mark it FIXME in >> the code now, and plug it when it's easy. > > Does this leak actually still exist now that you have a blk_unref() in > drive_del() (which is called during autodel) rather than do_drive_del()? Yes. The following hunk adds it: @@ -1813,11 +1811,11 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) * then we can just get rid of the block driver state right here. */ if (bdrv_get_attached_dev(bs)) { -bdrv_make_anon(bs); - +blk_hide_on_behalf_of_do_drive_del(blk); /* Further I/O must not pause the guest */ bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT); +/* FIXME bs->blk leaked when bs dies */ } else { drive_del(dinfo); } where blk_hide_on_behalf_of_do_drive_del() is from a few hunks further up: +/* + * Hide @blk. + * @blk must not have been hidden already. + * Make attached BlockDriverState, if any, anonymous. + * Once hidden, @blk is invisible to all functions that don't receive + * it as argument. For example, blk_by_name() won't return it. + * Strictly for use by do_drive_del(). + * TODO get rid of it! + */ +void blk_hide_on_behalf_of_do_drive_del(BlockBackend *blk) +{ +QTAILQ_REMOVE(&blk_backends, blk, link); +blk->name[0] = 0; +if (blk->bs) { +bdrv_make_anon(blk->bs); +} +} The net effect is just +QTAILQ_REMOVE(&blk_backends, blk, link); +blk->name[0] = 0; Admittedly not obvious: this messes with drive_del()! Let's follow the call chain. void blockdev_auto_del(BlockDriverState *bs) { DriveInfo *dinfo = drive_get_by_blockdev(bs); if (dinfo && dinfo->auto_del) { drive_del(dinfo); } } drive_get_by_blockdev() still returns the DriveInfo, as we haven't touched drives (we will in the next patch). However: void drive_del(DriveInfo *dinfo) { BlockBackend *blk = blk_by_name(dinfo->id); bdrv_unref(dinfo->bdrv); blk_unref(blk); } blk_by_name() returns null rather than the BB, because blk_hide_on_behalf_of_do_drive_del() already removed the BB from blk_backends. PATCH 06 plugs the leak: blockdev_auto_del() passes the BB straight to blk_unref(). But there's still a bug! Your review of v3 convinced me that the leak was already plugged in PATCH 04. Maybe it was in v3 (I didn't check again), but in v5, there's temporary breakage instead. I'll try to extend the test suite to cover it in v6.
Re: [Qemu-devel] [PATCH v5 04/23] block: Connect BlockBackend and DriveInfo
Kevin Wolf writes: > Am 02.10.2014 um 11:04 hat Markus Armbruster geschrieben: >> Make the BlockBackend own the DriveInfo. Change blockdev_init() to >> return the BlockBackend instead of the DriveInfo. >> >> Signed-off-by: Markus Armbruster >> Reviewed-by: Max Reitz > >> @@ -200,11 +202,11 @@ DriveInfo *drive_get_next(BlockInterfaceType type) >> >> DriveInfo *drive_get_by_blockdev(BlockDriverState *bs) >> { >> -DriveInfo *dinfo; >> +BlockBackend *blk; >> >> -QTAILQ_FOREACH(dinfo, &drives, next) { >> -if (dinfo->bdrv == bs) { >> -return dinfo; >> +for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { >> +if (blk == bs->blk) { >> +return blk_legacy_dinfo(blk); >> } >> } >> return NULL; > > In v3, I asked why you don't use bs->blk here. Apparently you understood > this as a suggestion to change the if condition from: > > if (blk_bs(blk) == bs) > > to: > > if (blk == bs->blk) > > Which isn't a wrong change, it just doesn't change a lot. What I really > meant is something like this, removing the loop: > > DriveInfo *drive_get_by_blockdev(BlockDriverState *bs) > { > return bs->blk ? blk_legacy_dinfo(bs->blk) : NULL; > } > > This would only behave differently if there were BlockBackends that can > be assigned to bs->blk, but aren't iterated by blk_next(). But such > BlockBackends don't exist, blk_next() includes all of them. Actually, there are: blk_hide_on_behalf_of_do_drive_del() removes from blk_backends without also zapping bs->blk. However, your version is *less* of a change, because it also doesn't remove from drives. In short: sold, thanks!
Re: [Qemu-devel] [PATCH 1/3] libqos: Remove PCI assumptions in virtio driver
On Thu, Sep 04, 2014 at 06:24:37PM +0200, Marc Marí wrote: > @@ -60,25 +60,25 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, > void *data) > *vpcidev = (QVirtioPCIDevice *)d; > } > > -static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, void *addr) > +static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr) > { > QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d; > -return qpci_io_readb(dev->pdev, addr); > +return qpci_io_readb(dev->pdev, (void *)addr); You do not need casts in C for void* to any pointer type or any pointer type to void*. Please drop them. > diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h > index 883f7ff..8f0e52a 100644 > --- a/tests/libqos/virtio-pci.h > +++ b/tests/libqos/virtio-pci.h > @@ -13,18 +13,18 @@ > #include "libqos/virtio.h" > #include "libqos/pci.h" > > -#define QVIRTIO_DEVICE_FEATURES 0x00 > -#define QVIRTIO_GUEST_FEATURES 0x04 > -#define QVIRTIO_QUEUE_ADDRESS 0x08 > -#define QVIRTIO_QUEUE_SIZE 0x0C > -#define QVIRTIO_QUEUE_SELECT0x0E > -#define QVIRTIO_QUEUE_NOTIFY0x10 > -#define QVIRTIO_DEVICE_STATUS 0x12 > -#define QVIRTIO_ISR_STATUS 0x13 > -#define QVIRTIO_MSIX_CONF_VECTOR0x14 > -#define QVIRTIO_MSIX_QUEUE_VECTOR 0x16 > -#define QVIRTIO_DEVICE_SPECIFIC_MSIX0x18 > -#define QVIRTIO_DEVICE_SPECIFIC_NO_MSIX 0x14 > +#define QVIRTIO_PCI_DEVICE_FEATURES 0x00 > +#define QVIRTIO_PCI_GUEST_FEATURES 0x04 > +#define QVIRTIO_PCI_QUEUE_ADDRESS 0x08 > +#define QVIRTIO_PCI_QUEUE_SIZE 0x0C > +#define QVIRTIO_PCI_QUEUE_SELECT0x0E > +#define QVIRTIO_PCI_QUEUE_NOTIFY0x10 > +#define QVIRTIO_PCI_DEVICE_STATUS 0x12 > +#define QVIRTIO_PCI_ISR_STATUS 0x13 > +#define QVIRTIO_PCI_MSIX_CONF_VECTOR0x14 > +#define QVIRTIO_PCI_MSIX_QUEUE_VECTOR 0x16 > +#define QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX0x18 > +#define QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX 0x14 This is a nice isolated change that could be made in a separate commit from the void* -> uint64_t change. It's a matter of style but makes code review easier since it is harder to review a patch that is performing multiple changes at once. pgpcUuCaeEotE.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
On Thu, Sep 18, 2014 at 06:17:48PM +0200, Paolo Bonzini wrote: > In the emergency last-minute patches of QEMU 2.1 we did two things: > > - fixed migration problems from 1.7 or 2.0 to 2.1 due to changes in > ACPI table sizes > > - ensured that future versions will not break migration compatibility > with 2.2 for reasonable configurations (with ACPI tables smaller > than a hundred kilobytes, roughly) > > However, this came at the cost of wasting 128 KB unconditionally on > even the smaller configuration, and we didn't provide a mechanism to > ensure compatibility with larger configurations. > > This series provides this mechanism. As mentioned early, the design > is to consider the SSDT immutable and versioned (together with other > non-AML tables such as HPET, TPMA and MADT, SRAT, MCFG, DMAR). > The DSDT instead can change more or less arbitrarily. To do this, > we add padding after the DSDT to allow for future growth. I'm not sure I got this one. Why is this padding more robust than the one we have now? > Once we do this, the size of the ACPI table fw_cfg "file" is constant > given a machine type and a command-line, so we do not need anymore the > larger 128KB padding. > > This is done in patches 4-6. > > However, there is another problem. As the ACPI tables grow, we need > to move the address at which linuxboot.bin loads the initrd. This > address is placed close to the end of memory, but it is QEMU that > tells linuxboot.bin where exactly the initrd is to be loaded. And > QEMU cannot really know how much high memory SeaBIOS will use, because > QEMU does not know the final e820 memory map. > > The solution would be to let linuxboot.bin parse the memory map and > ignore the suggested initrd base address, but that's tedious. In the > meanwhile, we can just assume that most of the need comes from the ACPI > tables (which is in fact true: patch 3 adds a fixed 32k extra just in > case) and dynamically resize the padding. > > This is what patches 1-3 do. The nice part is that they also remove > some differences between Xen and other accelerators. I would appreciate > Xen testing from interested people. > > Thanks, > > Paolo I'm not inclined to apply this for 2.2. Summarizing what you say, there are two issues around ACPI tables: - linuxboot uses FW CFG to for memory allocations, seabios ignores that, so they might conflict. Let's fix either linuxboot or seabios (or both!) and forget about it. - table size changes cause cross version migration issues this is really due to the fact we are using RAM to migrate ACPI tables. IMHO a more robust fix would be to allow RAM size to change during migration, or to avoid using RAM, switch to another type of object. So both issues have other solutions, and I think it's a good idea to focus on them for now. Also, I really would like to avoid having ACPI sizing-related issues for this release. The memory of 2.1.X pain is too fresh :) I'm not NACKing this patchset, but let's make some progress on the bigger issues listed above, then come back and address sizing as appropriate. Thanks! > > Paolo Bonzini (6): > pc: initialize fw_cfg earlier > pc: load the kernel after ACPI tables are built > pc: redo sizing of reserved high memory area for -kernel/-initrd > pc: introduce new ACPI table sizing algorithm > pc: go back to smaller ACPI tables > pc: clean up pre-2.1 compatibility code > > hw/i386/acpi-build.c | 23 +--- > hw/i386/pc.c | 72 > +-- > hw/i386/pc_piix.c| 32 ++ > hw/i386/pc_q35.c |7 ++-- > include/hw/i386/pc.h |4 ++ > 5 files changed, 66 insertions(+), 72 deletions(-)
Re: [Qemu-devel] [PATCH 0/2 V5] Virtual Machine Generation ID
On Wed, Sep 17, 2014 at 02:39:50PM +0300, Gal Hammer wrote: > Hi, > > A two parts patch to add a QEmu support for Microsoft's Virtual Machine > Generation ID device. > > The first one add a new ACPI directive which allow to use a 16-bytes > buffer in an ACPI table. This buffer is for storing the VM's UUID. > > The second is the ACPI tables changes and the actual device. > > Your comment are welcomed. > > Thanks, > > Gal. I applied this, thanks! > V5: - include the pre-compiled ASL file > - remove an empty line at end of files. > > V4: - Move device's description to SSDT table (dynamic build). > > V3: - Fix a typo in error message string. > - Move device's description from DSDT back to SSDT table. > > V2: - Remove "-uuid" command line parameter. > - Move device's description from SSDT to DSDT table. > - Add new "vmgenid" sysbus device. > > Gal Hammer (2): > i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive. > i386: Add a Virtual Machine Generation ID device. > > default-configs/i386-softmmu.mak | 1 + > default-configs/x86_64-softmmu.mak | 1 + > hw/i386/Makefile.objs | 2 +- > hw/i386/acpi-build.c | 39 +++ > hw/i386/ssdt-vmgenid.dsl | 63 > hw/i386/ssdt-vmgenid.hex.generated | 206 > + > hw/misc/Makefile.objs | 1 + > hw/misc/vmgenid.c | 84 +++ > include/hw/i386/pc.h | 3 + > scripts/acpi_extract.py| 23 +++-- > 10 files changed, 413 insertions(+), 10 deletions(-) > create mode 100644 hw/i386/ssdt-vmgenid.dsl > create mode 100644 hw/i386/ssdt-vmgenid.hex.generated > create mode 100644 hw/misc/vmgenid.c > > -- > 1.9.3 >
Re: [Qemu-devel] [PATCH 2/3] libqos: Add malloc generic
On Thu, Sep 04, 2014 at 06:24:38PM +0200, Marc Marí wrote: > This malloc is a basic interface implementation that works for any platform. > It should be replaced in the future for a real malloc implementation for each > of the platforms. > > Signed-off-by: Marc Marí > --- > tests/libqos/malloc-generic.c | 54 > + > tests/libqos/malloc-generic.h | 18 ++ > 2 files changed, 72 insertions(+) > create mode 100644 tests/libqos/malloc-generic.c > create mode 100644 tests/libqos/malloc-generic.h malloc-pc.c is the "real malloc implementation" you speak of. Just pc_alloc_init_flags() needs to be tweaked for the memory map of a particular platform. I'm not sure I see the point of this patch. Instead we should reuse malloc-pc.c. pgp_isk2_u9Oi.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 0/2 V5] Virtual Machine Generation ID
On Thu, Oct 02, 2014 at 03:12:53PM +0300, Michael S. Tsirkin wrote: > On Wed, Sep 17, 2014 at 02:39:50PM +0300, Gal Hammer wrote: > > Hi, > > > > A two parts patch to add a QEmu support for Microsoft's Virtual Machine > > Generation ID device. > > > > The first one add a new ACPI directive which allow to use a 16-bytes > > buffer in an ACPI table. This buffer is for storing the VM's UUID. > > > > The second is the ACPI tables changes and the actual device. > > > > Your comment are welcomed. > > > > Thanks, > > > > Gal. > > I applied this, thanks! Ooops wrong thread :( I will review this now though. > > V5: - include the pre-compiled ASL file > > - remove an empty line at end of files. > > > > V4: - Move device's description to SSDT table (dynamic build). > > > > V3: - Fix a typo in error message string. > > - Move device's description from DSDT back to SSDT table. > > > > V2: - Remove "-uuid" command line parameter. > > - Move device's description from SSDT to DSDT table. > > - Add new "vmgenid" sysbus device. > > > > Gal Hammer (2): > > i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive. > > i386: Add a Virtual Machine Generation ID device. > > > > default-configs/i386-softmmu.mak | 1 + > > default-configs/x86_64-softmmu.mak | 1 + > > hw/i386/Makefile.objs | 2 +- > > hw/i386/acpi-build.c | 39 +++ > > hw/i386/ssdt-vmgenid.dsl | 63 > > hw/i386/ssdt-vmgenid.hex.generated | 206 > > + > > hw/misc/Makefile.objs | 1 + > > hw/misc/vmgenid.c | 84 +++ > > include/hw/i386/pc.h | 3 + > > scripts/acpi_extract.py| 23 +++-- > > 10 files changed, 413 insertions(+), 10 deletions(-) > > create mode 100644 hw/i386/ssdt-vmgenid.dsl > > create mode 100644 hw/i386/ssdt-vmgenid.hex.generated > > create mode 100644 hw/misc/vmgenid.c > > > > -- > > 1.9.3 > >
Re: [Qemu-devel] [PATCH 1/2] i386: Add an ACPI_EXTRACT_NAME_BUFFER16 directive.
On Wed, Sep 17, 2014 at 02:39:51PM +0300, Gal Hammer wrote: > Add a 16-bytes buffer to allow storing a 128-bit UUID value in an > ACPI table. > > Signed-off-by: Gal Hammer > Reviewed-by: Paolo Bonzini This one looks good. Applied, thanks! > --- > scripts/acpi_extract.py | 23 ++- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/scripts/acpi_extract.py b/scripts/acpi_extract.py > index 22ea468..10c1ffb 100755 > --- a/scripts/acpi_extract.py > +++ b/scripts/acpi_extract.py > @@ -139,13 +139,16 @@ def aml_name_string(offset): > offset += 1 > return offset; > > -# Given data offset, find 8 byte buffer offset > -def aml_data_buffer8(offset): > -#0x08 NameOp NameString DataRef > -expect = [0x11, 0x0B, 0x0A, 0x08] > +# Given data offset, find variable length byte buffer offset > +def aml_data_buffer(offset, length): > +#0x11 PkgLength BufferSize ByteList > +if (length > 63): > +die( "Name offset 0x%x: expected a one byte PkgLength (length<=63)" % > + (offset)); > +expect = [0x11, length+3, 0x0A, length] > if (aml[offset:offset+4] != expect): > die( "Name offset 0x%x: expected %s actual %s" % > - (offset, aml[offset:offset+4], expect)) > + (offset, expect, aml[offset:offset+4])) > return offset + len(expect) > > # Given data offset, find dword const offset > @@ -172,9 +175,9 @@ def aml_data_byte_const(offset): > (offset, aml[offset])); > return offset + 1; > > -# Find name'd buffer8 > -def aml_name_buffer8(offset): > -return aml_data_buffer8(aml_name_string(offset) + 4) > +# Find name'd buffer > +def aml_name_buffer(offset, length): > +return aml_data_buffer(aml_name_string(offset) + 4, length) > > # Given name offset, find dword const offset > def aml_name_dword_const(offset): > @@ -308,7 +311,9 @@ for i in range(len(asl)): > output[array] = aml > continue > if (directive == "ACPI_EXTRACT_NAME_BUFFER8"): > -offset = aml_name_buffer8(offset) > +offset = aml_name_buffer(offset, 8) > +elif (directive == "ACPI_EXTRACT_NAME_BUFFER16"): > +offset = aml_name_buffer(offset, 16) > elif (directive == "ACPI_EXTRACT_NAME_DWORD_CONST"): > offset = aml_name_dword_const(offset) > elif (directive == "ACPI_EXTRACT_NAME_WORD_CONST"): > -- > 1.9.3 >
Re: [Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect
Il 01/10/2014 23:09, miny...@acm.org ha scritto: > From: Corey Minyard > > Move all socket configuration to qmp_chardev_open_socket(). > qemu_chr_open_socket_fd() just opens the socket. This is getting ready > for the reconnect code, which will call open_sock_fd() on a reconnect > attempt. > > Signed-off-by: Corey Minyard > Reviewed-by: Paolo Bonzini > --- > qemu-char.c | 118 > ++-- > 1 file changed, 68 insertions(+), 50 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index f9d2a02..7928a4b 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2891,13 +2891,11 @@ static void tcp_chr_close(CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > -static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, > -bool is_listen, bool > is_telnet, > -bool is_waitconnect, > -Error **errp) > +static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd, > + bool is_listen, bool is_telnet, > + Error **errp) > { > -CharDriverState *chr = NULL; > -TCPCharDriver *s = NULL; > +TCPCharDriver *s = chr->opaque; > char host[NI_MAXHOST], serv[NI_MAXSERV]; > const char *left = "", *right = ""; > struct sockaddr_storage ss; > @@ -2905,26 +2903,14 @@ static CharDriverState *qemu_chr_open_socket_fd(int > fd, bool do_nodelay, > > memset(&ss, 0, ss_len); > if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) { > +closesocket(fd); > error_setg_errno(errp, errno, "getsockname"); > -return NULL; > +return false; > } > > -chr = qemu_chr_alloc(); > -s = g_malloc0(sizeof(TCPCharDriver)); > - > -s->connected = 0; > -s->fd = -1; > -s->listen_fd = -1; > -s->read_msgfds = 0; > -s->read_msgfds_num = 0; > -s->write_msgfds = 0; > -s->write_msgfds_num = 0; > - > -chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE); > switch (ss.ss_family) { > #ifndef _WIN32 > case AF_UNIX: > -s->is_unix = 1; > snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s", > ((struct sockaddr_un *)(&ss))->sun_path, > is_listen ? ",server" : ""); > @@ -2935,7 +2921,6 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, > bool do_nodelay, > right = "]"; > /* fall through */ > case AF_INET: > -s->do_nodelay = do_nodelay; > getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host), > serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV); > snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s", > @@ -2945,25 +2930,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int > fd, bool do_nodelay, > break; > } > > -chr->opaque = s; > -chr->chr_write = tcp_chr_write; > -chr->chr_sync_read = tcp_chr_sync_read; > -chr->chr_close = tcp_chr_close; > -chr->get_msgfds = tcp_get_msgfds; > -chr->set_msgfds = tcp_set_msgfds; > -chr->chr_add_client = tcp_chr_add_client; > -chr->chr_add_watch = tcp_chr_add_watch; > -chr->chr_update_read_handler = tcp_chr_update_read_handler; > -/* be isn't opened until we get a connection */ > -chr->explicit_be_open = true; > - > if (is_listen) { > s->listen_fd = fd; > s->listen_chan = io_channel_from_socket(s->listen_fd); > -s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, > tcp_chr_accept, chr); > -if (is_telnet) { > -s->do_telnetopt = 1; > -} > +s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, > + tcp_chr_accept, chr); > } else { > s->connected = 1; > s->fd = fd; > @@ -2972,15 +2943,28 @@ static CharDriverState *qemu_chr_open_socket_fd(int > fd, bool do_nodelay, > tcp_chr_connect(chr); > } > > -if (is_listen && is_waitconnect) { > -fprintf(stderr, "QEMU waiting for connection on: %s\n", > -chr->filename); > -tcp_chr_accept(s->listen_chan, G_IO_IN, chr); > -qemu_set_nonblock(s->listen_fd); > -} > return chr; You should return true here rather than chr. Paolo > } > > +static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress > *addr, > +bool is_listen, bool is_telnet, > +Error **errp) > +{ > +int fd; > + > +if (is_listen) { > +fd = socket_listen(addr, errp); > +} else { > +fd = socket_connect(addr, errp, NULL, NULL); > +} > +if (fd < 0) { > +return false; > +} > + > +return qemu_chr_finish_socket_connection(ch
Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
Il 01/10/2014 23:09, miny...@acm.org ha scritto: > From: Corey Minyard > > This way we can tell if the socket is connected or not. It also splits > the string conversions out into separate functions to make this more > convenient. > > Signed-off-by: Corey Minyard > Reviewed-by: Paolo Bonzini > --- > qemu-char.c | 102 > > 1 file changed, 69 insertions(+), 33 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index cd98911..b118e88 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -117,6 +117,60 @@ static void qapi_copy_SocketAddress(SocketAddress > **p_dest, > qobject_decref(obj); > } > > +static int SocketAddress_to_str(char *dest, int max_len, > +const char *prefix, SocketAddress *addr, > +bool is_listen, bool is_telnet) > +{ > +switch (addr->kind) { > +case SOCKET_ADDRESS_KIND_INET: > +return snprintf(dest, max_len, "%s%s:%s:%s%s", prefix, > +is_telnet ? "telnet" : "tcp", addr->inet->host, > +addr->inet->port, is_listen ? ",server" : ""); > +break; > +case SOCKET_ADDRESS_KIND_UNIX: > +return snprintf(dest, max_len, "%sunix:%s%s", prefix, > +addr->q_unix->path, is_listen ? ",server" : ""); > +break; > +case SOCKET_ADDRESS_KIND_FD: > +return snprintf(dest, max_len, "%sfd:%s%s", prefix, addr->fd->str, > +is_listen ? ",server" : ""); > +break; > +default: > +abort(); > +} > +} > + > +static int sockaddr_to_str(char *dest, int max_len, > + struct sockaddr_storage *ss, socklen_t ss_len, > + bool is_listen, bool is_telnet) > +{ > +char host[NI_MAXHOST], serv[NI_MAXSERV]; > +const char *left = "", *right = ""; > + > +switch (ss->ss_family) { > +#ifndef _WIN32 > +case AF_UNIX: > +return snprintf(dest, max_len, "unix:%s%s", > +((struct sockaddr_un *)(ss))->sun_path, > +is_listen ? ",server" : ""); > +#endif > +case AF_INET6: > +left = "["; > +right = "]"; > +/* fall through */ > +case AF_INET: > +getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host), > +serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV); > +return snprintf(dest, max_len, "%s:%s%s%s:%s%s", > +is_telnet ? "telnet" : "tcp", > +left, host, right, serv, > +is_listen ? ",server" : ""); > + > +default: > +return snprintf(dest, max_len, "unknown"); > +} > +} > + > /***/ > /* character device */ > > @@ -2727,6 +2781,8 @@ static void tcp_chr_disconnect(CharDriverState *chr) > s->chan = NULL; > closesocket(s->fd); > s->fd = -1; > +SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, > + "disconnected:", s->addr, s->is_listen, > s->is_telnet); > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > @@ -2798,6 +2854,17 @@ static void tcp_chr_connect(void *opaque) > { > CharDriverState *chr = opaque; > TCPCharDriver *s = chr->opaque; > +struct sockaddr_storage ss; > +socklen_t ss_len = sizeof(ss); > + > +memset(&ss, 0, ss_len); > +if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) { > +snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, > + "Error in getsockname: %s\n", strerror(errno)); > +} else { > +sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len, > +s->is_listen, s->is_telnet); > +} > > s->connected = 1; > if (s->chan) { > @@ -2932,39 +2999,6 @@ static bool > qemu_chr_finish_socket_connection(CharDriverState *chr, int fd, >Error **errp) As a side effect, errp cannot be set anymore, and the function will always return true. Please make it return void, and drop the last argument. Paolo > { > TCPCharDriver *s = chr->opaque; > -char host[NI_MAXHOST], serv[NI_MAXSERV]; > -const char *left = "", *right = ""; > -struct sockaddr_storage ss; > -socklen_t ss_len = sizeof(ss); > - > -memset(&ss, 0, ss_len); > -if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) { > -closesocket(fd); > -error_setg_errno(errp, errno, "getsockname"); > -return false; > -} > - > -switch (ss.ss_family) { > -#ifndef _WIN32 > -case AF_UNIX: > -snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s", > - ((struct sockaddr_un *)(&ss))->sun_path, > - s->is_listen ? ",server" : ""); > -break; > -#endif > -case AF_INET6: > -left = "["; > -right = "]"; > -
Re: [Qemu-devel] [PULL 00/10] vga: cleanups, prepare for endianness switching
On 2 October 2014 08:44, Gerd Hoffmann wrote: > Hi, > > vga cleanup patches finally on the path upstream ;) > > please pull, > Gerd > > The following changes since commit 29429c7244c73eefada3d0ec6dd30c5698782d08: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20140929' into staging (2014-09-30 > 11:02:06 +0100) > > are available in the git repository at: > > > git://git.kraxel.org/qemu tags/pull-vga-20141002-1 > > for you to fetch changes up to c3b10605147f9113b8b157d7226d3e215184bc0e: > > vga: Add endian to vmstate (2014-09-30 13:34:09 +0200) > > > vga: cleanups, prepare for endianness switching > > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 3/3] libqos: Add virtio MMIO support
On Thu, Sep 04, 2014 at 06:24:39PM +0200, Marc Marí wrote: > +QVirtioMMIODevice *qvirtio_mmio_init_device(uint64_t addr, uint32_t > page_size) > +{ > +QVirtioMMIODevice *dev; > +union { uint32_t magic; char bytes[5]; } magic_value; > +dev = g_malloc0(sizeof(*dev)); > +magic_value.bytes[4] = '\0'; > + > +magic_value.magic = readl(addr + QVIRTIO_MMIO_MAGIC_VALUE); > +g_assert_cmpstr(magic_value.bytes, ==, "virt"); Endianness trouble here. Please do what the Linux virtio_mmio driver does: magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE); if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) { pgp2zriPbqqDa.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets
Il 01/10/2014 23:09, miny...@acm.org ha scritto: > From: Corey Minyard > > Adds a "reconnect" option to socket backends that gives a reconnect > timeout. This only applies to client sockets. If the other end > of a socket closes the connection, qemu will attempt to reconnect > after the given number of seconds. > > Signed-off-by: Corey Minyard > --- > qapi-schema.json | 15 ++ > qemu-char.c | 88 > > qemu-options.hx | 20 - > 3 files changed, 105 insertions(+), 18 deletions(-) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 4bfaf20..148097b 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2651,14 +2651,19 @@ > # @nodelay: #optional set TCP_NODELAY socket option (default: false) > # @telnet: #optional enable telnet protocol on server > # sockets (default: false) > +# @reconnect: #optional For a client socket, if a socket is disconnected, > +# then attempt a reconnect after the given number of seconds. > +# Setting this to zero disables this function. (default: 0) > +# (Since: 2.2) > # > # Since: 1.4 > ## > -{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress', > - '*server' : 'bool', > - '*wait': 'bool', > - '*nodelay' : 'bool', > - '*telnet' : 'bool' } } > +{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress', > + '*server': 'bool', > + '*wait' : 'bool', > + '*nodelay' : 'bool', > + '*telnet': 'bool', > + '*reconnect' : 'int' } } > > ## > # @ChardevUdp: > diff --git a/qemu-char.c b/qemu-char.c > index b118e88..f33173c 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2501,8 +2501,20 @@ typedef struct { > SocketAddress *addr; > bool is_listen; > bool is_telnet; > + > +guint reconnect_timer; > +int64_t reconnect_time; > } TCPCharDriver; > > +static gboolean socket_reconnect_timeout(gpointer opaque); > + > +static void qemu_chr_socket_restart_timer(CharDriverState *chr) > +{ > +TCPCharDriver *s = chr->opaque; Please assert that s->connected == 0 here. > +s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time, > + socket_reconnect_timeout, > chr); > +} > + > static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void > *opaque); > > #ifndef _WIN32 > @@ -2784,6 +2796,9 @@ static void tcp_chr_disconnect(CharDriverState *chr) > SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, > "disconnected:", s->addr, s->is_listen, > s->is_telnet); > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > +if (s->reconnect_time) { > +qemu_chr_socket_restart_timer(chr); > +} > } > > static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void > *opaque) > @@ -2964,6 +2979,10 @@ static void tcp_chr_close(CharDriverState *chr) > TCPCharDriver *s = chr->opaque; > int i; > > +if (s->reconnect_timer) { > +g_source_remove(s->reconnect_timer); > +s->reconnect_timer = 0; > +} > qapi_free_SocketAddress(s->addr); > if (s->fd >= 0) { > remove_fd_in_watch(chr); > @@ -3013,7 +3032,28 @@ static bool > qemu_chr_finish_socket_connection(CharDriverState *chr, int fd, > tcp_chr_connect(chr); > } > > -return chr; > +return true; As mentioned in patch 2, this should be done there. > +} > + > +static void qemu_chr_socket_connected(int fd, void *opaque) > +{ > +CharDriverState *chr = opaque; > +TCPCharDriver *s = chr->opaque; > +Error *err = NULL; > + > +if (fd >= 0) { > +if (qemu_chr_finish_socket_connection(chr, fd, &err)) { If the errp argument is removed in qemu_chr_finish_socket_connection... > +return; > +} > +if (err) { > +error_report("%s", error_get_pretty(err)); > +error_free(err); > +} > +closesocket(fd); > +} > + > +s->connected = 0; ... and we are sure that s->connected is already 0, this function can be simply CharDriverState *chr = opaque; if (fd < 0) { qemu_chr_socket_restart_timer(chr); return; } qemu_chr_finish_socket_connection(chr, fd); I have no other comment. Everything here is basically rippling from the observation that errp is unused after patch 4; sorry for not noticing that earlier. Paolo > +qemu_chr_socket_restart_timer(chr); > } > > static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp) > @@ -3023,7 +3063,10 @@ static bool qemu_chr_open_socket_fd(CharDriverState > *chr, Error
Re: [Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files
On Thu, Oct 02, 2014 at 08:49:12AM +0200, Markus Armbruster wrote: > Eduardo Habkost writes: > > > With this, code that uses symbols from qemu-file.c don't need to bring extra > > dependencies because of the actual QEMUFile operation implementations. > > Each case of omitting the "extra dependencies" should be visible in > makefiles as "prerequisites include qemu-file.o, but not all > qemu-file-*.o". I can see one: tests/Makefile has just > qemu-file-unix.o, but not qemu-file-stdio.o. Out of curiosity: will > there be more? Yes, tests/test-x86-cpu[1] will only need qemu-file.o. That was the original motivation for this series. [1] See Subject: [PATCH v2 0/7] Target-specific unit test support, add unit tests for target-i386/cpu.c code -- Eduardo
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
Il 02/10/2014 14:11, Michael S. Tsirkin ha scritto: > On Thu, Sep 18, 2014 at 06:17:48PM +0200, Paolo Bonzini wrote: >> In the emergency last-minute patches of QEMU 2.1 we did two things: >> >> - fixed migration problems from 1.7 or 2.0 to 2.1 due to changes in >> ACPI table sizes >> >> - ensured that future versions will not break migration compatibility >> with 2.2 for reasonable configurations (with ACPI tables smaller >> than a hundred kilobytes, roughly) >> >> However, this came at the cost of wasting 128 KB unconditionally on >> even the smaller configuration, and we didn't provide a mechanism to >> ensure compatibility with larger configurations. >> >> This series provides this mechanism. As mentioned early, the design >> is to consider the SSDT immutable and versioned (together with other >> non-AML tables such as HPET, TPMA and MADT, SRAT, MCFG, DMAR). >> The DSDT instead can change more or less arbitrarily. To do this, >> we add padding after the DSDT to allow for future growth. > > I'm not sure I got this one. > Why is this padding more robust than the one we have now? Because the current one applies to all ACPI tables. After this patch, the padding only applies to the "fixed" tables (FACS, FADT, DSDT). With the old algorithm, a small change in the DSDT, combined with the "right" size of the MADT/SSDT, could cause a change in the rounded size. With the new algorithm, any change in the DSDT will either break migration or will not, independent of the content of the SSDT. Example with the old algorithm (up to 2.0): old version of QEMU new version of QEMU fixed table size 4000 4100 variable table size4100 4100 total size 8100 8100 4k-rounded size8192 12288 In 2.1 we changed the rounding to 128k, but the fundamental problem remains and that is why we warn for ACPI tables whose size is above 64k. The same example with new algorithm: old version of QEMU new version of QEMU fixed table size 4000 4100 padded to 16384 16384 variable table size4100 4100 total size17484 17484 4k-rounded size 20480 20480 Paolo >> Once we do this, the size of the ACPI table fw_cfg "file" is constant >> given a machine type and a command-line, so we do not need anymore the >> larger 128KB padding. >> >> This is done in patches 4-6. >> >> However, there is another problem. As the ACPI tables grow, we need >> to move the address at which linuxboot.bin loads the initrd. This >> address is placed close to the end of memory, but it is QEMU that >> tells linuxboot.bin where exactly the initrd is to be loaded. And >> QEMU cannot really know how much high memory SeaBIOS will use, because >> QEMU does not know the final e820 memory map. >> >> The solution would be to let linuxboot.bin parse the memory map and >> ignore the suggested initrd base address, but that's tedious. In the >> meanwhile, we can just assume that most of the need comes from the ACPI >> tables (which is in fact true: patch 3 adds a fixed 32k extra just in >> case) and dynamically resize the padding. >> >> This is what patches 1-3 do. The nice part is that they also remove >> some differences between Xen and other accelerators. I would appreciate >> Xen testing from interested people. >> >> Thanks, >> >> Paolo > > > I'm not inclined to apply this for 2.2. > > Summarizing what you say, there are two issues around ACPI tables: > - linuxboot uses FW CFG to for memory allocations, > seabios ignores that, so they might conflict. > Let's fix either linuxboot or seabios (or both!) > and forget about it. > > - table size changes cause cross version migration issues > this is really due to the fact we are using RAM > to migrate ACPI tables. > IMHO a more robust fix would be to allow RAM size to change > during migration, or to avoid using RAM, switch to another type of > object. > > So both issues have other solutions, and I think it's a good > idea to focus on them for now. > Also, I really would like to avoid having ACPI sizing-related > issues for this release. The memory of 2.1.X pain is too fresh :) > I'm not NACKing this patchset, but let's > make some progress on the bigger issues listed above, then come > back and address sizing as appropriate. > > Thanks! > >> >> Paolo Bonzini (6): >> pc: initialize fw_cfg earlier >> pc: load the kernel after ACPI tables are built >> pc: redo sizing of reserved high memory area for -kernel/-initrd >> pc: introduce new ACPI table sizing algorithm >> pc: go back to smaller ACPI tables >> pc: clean up pre-2.1 compatibility code >> >> hw/i386/acpi-build.c | 23 +--- >> hw/i386/pc.c | 72 >>
Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
On Wed, Sep 17, 2014 at 02:39:52PM +0300, Gal Hammer wrote: > Based on Microsoft's sepecifications (paper can be dowloaded from > http://go.microsoft.com/fwlink/?LinkId=260709), add a device > description to the SSDT ACPI table. > > The GUID is set using a new "vmgenid" device. > > Signed-off-by: Gal Hammer > Reviewed-By: Igor Mammedov > --- > default-configs/i386-softmmu.mak | 1 + > default-configs/x86_64-softmmu.mak | 1 + > hw/i386/Makefile.objs | 2 +- > hw/i386/acpi-build.c | 39 +++ > hw/i386/ssdt-vmgenid.dsl | 63 > hw/i386/ssdt-vmgenid.hex.generated | 206 > + > hw/misc/Makefile.objs | 1 + > hw/misc/vmgenid.c | 84 +++ > include/hw/i386/pc.h | 3 + Patch scripts/update-acpi.sh as well please. > 9 files changed, 399 insertions(+), 1 deletion(-) > create mode 100644 hw/i386/ssdt-vmgenid.dsl > create mode 100644 hw/i386/ssdt-vmgenid.hex.generated > create mode 100644 hw/misc/vmgenid.c > > diff --git a/default-configs/i386-softmmu.mak > b/default-configs/i386-softmmu.mak > index 8e08841..bd33c75 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y > CONFIG_ICC_BUS=y > CONFIG_PVPANIC=y > CONFIG_MEM_HOTPLUG=y > +CONFIG_VMGENID=y > diff --git a/default-configs/x86_64-softmmu.mak > b/default-configs/x86_64-softmmu.mak > index 66557ac..006fc7c 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y > CONFIG_ICC_BUS=y > CONFIG_PVPANIC=y > CONFIG_MEM_HOTPLUG=y > +CONFIG_VMGENID=y > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > index 9d419ad..cd1beb3 100644 > --- a/hw/i386/Makefile.objs > +++ b/hw/i386/Makefile.objs > @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c > hw/i386/acpi-dsdt.hex \ > hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \ > hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \ > hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \ > - hw/i386/ssdt-tpm.hex > + hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex > > iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \ > ; then echo "$(2)"; else echo "$(3)"; fi ;) > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index a313321..72d5a88 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo { > const unsigned char *dsdt_code; > unsigned dsdt_size; > uint16_t pvpanic_port; > +bool vm_generation_id_set; > +uint8_t vm_generation_id[16]; > } AcpiMiscInfo; > > typedef struct AcpiBuildPciBusHotplugState { > @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > info->has_hpet = hpet_find(); > info->has_tpm = tpm_find(); > info->pvpanic_port = pvpanic_port(); > +info->vm_generation_id_set = vm_generation_id(info->vm_generation_id); > } > > static void acpi_get_pci_info(PcPciInfo *info) > @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val) > #include "hw/i386/ssdt-misc.hex" > #include "hw/i386/ssdt-pcihp.hex" > #include "hw/i386/ssdt-tpm.hex" > +#include "hw/i386/ssdt-vmgenid.hex" > > static void > build_append_notify_method(GArray *device, const char *name, > @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker) > memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml)); > } > > +static void > +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info) > +{ > +int vgid_start = table_data->len; > +void *vgid_ptr; > +uint8_t *vm_gid_ptr; > +uint32_t vm_gid_physical_address; > + > +vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml)); > +memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml)); > + > +vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml), > + *ssdt_acpi_vm_gid, > + sizeof(info->vm_generation_id)); > +memcpy(vm_gid_ptr, info->vm_generation_id, > + sizeof(info->vm_generation_id)); > + > +bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > + ACPI_BUILD_TABLE_FILE, > + table_data, > + vgid_ptr + *ssdt_acpi_vm_gid_addr, > + sizeof(uint32_t)); > + > +vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid; > +ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml), > + *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address); > + > +build_header(linker, table_data, vgid_ptr, "SSDT", > + sizeof(ssdt_vmgenid_aml), 1); > +} > + > typedef enum { > MEM_AFFINITY_NOFLAGS = 0, > MEM_AFFINITY_ENABLED = (1 << 0), >
Re: [Qemu-devel] [PATCH v5] vmdk: Fix integer overflow in offset calculation
On Tue, Sep 23, 2014 at 09:56:21AM +0800, Fam Zheng wrote: > This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster > allocation). > > $ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' > write failed: Invalid argument > > Reported-by: Mark Cave-Ayland > Reviewed-by: Max Reitz > Signed-off-by: Fam Zheng > > --- > v5: Fix group file and output reference. (Max) > v4: Fix typo in file header: 1014 -> 2014. > v3: A new case 105 instead of embedding in 005. (Max) > --- > block/vmdk.c | 2 +- > tests/qemu-iotests/105 | 70 > ++ > tests/qemu-iotests/105.out | 21 ++ > tests/qemu-iotests/group | 1 + > 4 files changed, 93 insertions(+), 1 deletion(-) > create mode 100755 tests/qemu-iotests/105 > create mode 100644 tests/qemu-iotests/105.out Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgp9faHmafX0k.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 3/6] pc/vl: Add units-per-default-bus property
On Wed, Oct 01, 2014 at 02:19:26PM -0400, John Snow wrote: > This patch adds the 'units_per_default_bus' property which > allows individual boards to declare their desired > index => (bus,unit) mapping for their default HBA, so that > boards such as Q35 can specify that its default if_ide HBA, > AHCI, only accepts one unit per bus. > > This property only overrides the mapping for drives matching > the block_default_type interface. > > This patch also adds this property to *all* past and present > Q35 machine types. This retroactive addition is justified > because the previous erroneous index=>(bus,unit) mappings > caused by lack of such a property were not utilized due to > lack of initialization code in the Q35 init routine. > > Further, semantically, the Q35 board type has always had the > property that its default HBA, AHCI, only accepts one unit per > bus. The new code added to add devices to drives relies upon > the accuracy of this mapping. Thus, the property is applied > retroactively to reduce complexity of allowing IDE HBAs with > different units per bus. > > Examples: > > Prior to this patch, all IDE HBAs were assumed to use 2 units > per bus (Master, Slave). When using Q35 and AHCI, however, we > only allow one unit per bus. > > -hdb foo.qcow2 would become index=1, or bus=0,unit=1. > -hdd foo.qcow2 would become index=3, or bus=1,unit=1. > -drive file=foo.qcow2,index=5 becomes bus=2,unit=1. > > These are invalid for AHCI. They now become, under Q35 only: > > -hdb foo.qcow2 --> index=1, bus=1, unit=0. > -hdd foo.qcow2 --> index=3, bus=3, unit=0. > -drive file=foo.qcow2,index=5 --> bus=5,unit=0. > > The mapping is adjusted based on the fact that the default IF > for the Q35 machine type is IF_IDE, and units-per-default-bus > overrides the IDE mapping from its default of 2 units per bus > to just 1 unit per bus. > > Signed-off-by: John Snow Reviewed-by: Michael S. Tsirkin > --- > hw/i386/pc.c| 1 + > hw/i386/pc_q35.c| 3 ++- > include/hw/boards.h | 2 ++ > vl.c| 8 > 4 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 82a7daa..d045e8b 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1524,6 +1524,7 @@ static void pc_generic_machine_class_init(ObjectClass > *oc, void *data) > mc->hot_add_cpu = qm->hot_add_cpu; > mc->kvm_type = qm->kvm_type; > mc->block_default_type = qm->block_default_type; > +mc->units_per_default_bus = qm->units_per_default_bus; > mc->max_cpus = qm->max_cpus; > mc->no_serial = qm->no_serial; > mc->no_parallel = qm->no_parallel; > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index d4a907c..b28ddbb 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -344,7 +344,8 @@ static void pc_q35_init_1_4(MachineState *machine) > #define PC_Q35_MACHINE_OPTIONS \ > PC_DEFAULT_MACHINE_OPTIONS, \ > .desc = "Standard PC (Q35 + ICH9, 2009)", \ > -.hot_add_cpu = pc_hot_add_cpu > +.hot_add_cpu = pc_hot_add_cpu, \ > +.units_per_default_bus = 1 > > #define PC_Q35_2_2_MACHINE_OPTIONS \ > PC_Q35_MACHINE_OPTIONS, \ > diff --git a/include/hw/boards.h b/include/hw/boards.h > index dfb6718..663f16a 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -28,6 +28,7 @@ struct QEMUMachine { > QEMUMachineHotAddCPUFunc *hot_add_cpu; > QEMUMachineGetKvmtypeFunc *kvm_type; > BlockInterfaceType block_default_type; > +int units_per_default_bus; > int max_cpus; > unsigned int no_serial:1, > no_parallel:1, > @@ -86,6 +87,7 @@ struct MachineClass { > int (*kvm_type)(const char *arg); > > BlockInterfaceType block_default_type; > +int units_per_default_bus; > int max_cpus; > unsigned int no_serial:1, > no_parallel:1, > diff --git a/vl.c b/vl.c > index 6500472..940b149 100644 > --- a/vl.c > +++ b/vl.c > @@ -1588,6 +1588,7 @@ static void machine_class_init(ObjectClass *oc, void > *data) > mc->hot_add_cpu = qm->hot_add_cpu; > mc->kvm_type = qm->kvm_type; > mc->block_default_type = qm->block_default_type; > +mc->units_per_default_bus = qm->units_per_default_bus; > mc->max_cpus = qm->max_cpus; > mc->no_serial = qm->no_serial; > mc->no_parallel = qm->no_parallel; > @@ -4378,6 +4379,13 @@ int main(int argc, char **argv, char **envp) > blk_mig_init(); > ram_mig_init(); > > +/* If the currently selected machine wishes to override the units-per-bus > + * property of its default HBA interface type, do so now. */ > +if (machine_class->units_per_default_bus) { > +override_max_devs(machine_class->block_default_type, > + machine_class->units_per_default_bus); > +} > + > /* open the virtual block devices */ > if (snapshot) > qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, > NULL, 0); > --
Re: [Qemu-devel] [PATCH v3 5/6] qtest/bios-tables: Correct Q35 command line
On Wed, Oct 01, 2014 at 02:19:28PM -0400, John Snow wrote: > If the Q35 board types are to begin recognizing > and decoding syntactic sugar for drive/device > declarations, then workarounds found within > the qtests suite need to be adjusted to prevent > any test failures after the fix. > > bios-tables-test improperly uses this cli: > -drive file=etc,id=hd -device ide-hd,drive=hd > > Which will create a drive and device due to > the lack of specifying if=none. Then, it will > attempt to create a second device and fail. > > This patch corrects this test to always use > the full, non-sugared -device/-drive syntax > for both PC and Q35. > > Signed-off-by: John Snow Reviewed-by: Michael S. Tsirkin > --- > tests/bios-tables-test.c | 10 -- > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > index 602932b..9e4d205 100644 > --- a/tests/bios-tables-test.c > +++ b/tests/bios-tables-test.c > @@ -714,14 +714,12 @@ static void test_acpi_one(const char *params, test_data > *data) > uint8_t signature_high; > uint16_t signature; > int i; > -const char *device = ""; > > -if (!g_strcmp0(data->machine, MACHINE_Q35)) { > -device = ",id=hd -device ide-hd,drive=hd"; > -} > +args = g_strdup_printf("-net none -display none %s " > + "-drive id=hd0,if=none,file=%s " > + "-device ide-hd,drive=hd0 ", > + params ? params : "", disk); > > -args = g_strdup_printf("-net none -display none %s -drive file=%s%s,", > - params ? params : "", disk, device); > qtest_start(args); > > /* Wait at most 1 minute */ > -- > 1.9.3
Re: [Qemu-devel] [PATCH v3 6/6] q35/ahci: Pick up -cdrom and -hda options
On Wed, Oct 01, 2014 at 02:19:29PM -0400, John Snow wrote: > This patch implements the backend for the Q35 board > for us to be able to pick up and use drives defined > by the -cdrom, -hda, or -drive if=ide shorthand options. > > Signed-off-by: John Snow Reviewed-by: Michael S. Tsirkin > --- > hw/i386/pc_q35.c | 4 > hw/ide/ahci.c| 15 +++ > hw/ide/ahci.h| 2 ++ > 3 files changed, 21 insertions(+) > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index b28ddbb..bb0dc8e 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -86,6 +86,7 @@ static void pc_q35_init(MachineState *machine) > DeviceState *icc_bridge; > PcGuestInfo *guest_info; > ram_addr_t lowmem; > +DriveInfo *hd[MAX_SATA_PORTS]; > > /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory > * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping > @@ -253,6 +254,9 @@ static void pc_q35_init(MachineState *machine) > true, "ich9-ahci"); > idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0"); > idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1"); > +g_assert_cmpint(MAX_SATA_PORTS, ==, ICH_AHCI(ahci)->ahci.ports); > +ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports); > +ahci_ide_create_devs(ahci, hd); > > if (usb_enabled(false)) { > /* Should we create 6 UHCI according to ich9 spec? */ > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 8978643..063730e 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -1419,3 +1419,18 @@ static void sysbus_ahci_register_types(void) > } > > type_init(sysbus_ahci_register_types) > + > +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd) > +{ > +AHCIPCIState *d = ICH_AHCI(dev); > +AHCIState *ahci = &d->ahci; > +int i; > + > +for (i = 0; i < ahci->ports; i++) { > +if (hd[i] == NULL) { > +continue; > +} > +ide_create_drive(&ahci->dev[i].port, 0, hd[i]); > +} > + > +} > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h > index 1543df7..e223258 100644 > --- a/hw/ide/ahci.h > +++ b/hw/ide/ahci.h > @@ -332,4 +332,6 @@ void ahci_uninit(AHCIState *s); > > void ahci_reset(AHCIState *s); > > +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd); > + > #endif /* HW_IDE_AHCI_H */ > -- > 1.9.3
Re: [Qemu-devel] [PATCH 0/2] make check-block/qemu-iotests fixes
On Tue, Sep 30, 2014 at 01:27:08PM +0200, Kevin Wolf wrote: > Kevin Wolf (2): > make check-block: Use default cache modes > qemu-iotests: Fix supported cache modes for 052 > > tests/qemu-iotests-quick.sh | 2 +- > tests/qemu-iotests/052 | 5 +++-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > -- > 1.8.3.1 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgpIStdzxaIRF.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] util: Emancipate id_wellformed() from QemuOpts
On Tue, Sep 30, 2014 at 01:59:30PM +0200, Markus Armbruster wrote: > +bool id_wellformed(const char *id) > +{ > +int i; > + > +if (!qemu_isalpha(id[0])) { > +return 0; false would be a bit nicer since the other return cases use true/false. pgpWncgsK8jId.pgp Description: PGP signature
Re: [Qemu-devel] IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpts)
On Wed, Oct 01, 2014 at 02:33:47PM +0200, Markus Armbruster wrote: > Markus Armbruster writes: This discussion seems orthogonal to your patch. But I'm not applying it yet to give more time for discussion/review of the patch. > Is mangling array-ness into the name really a good idea? Isn't this > type matter, not name matter? I agree. It's nasty to hack the array selector into the name and will probably cause us pain down the line. > Backtracking a bit... Unlike QMP object-add, -object ) and HMP > object-add use QemuOpts. See object_create(), commit 68d98d3 "vl: add > -object option to create QOM objects from the command line", and > hmp_object_add(), commit cff8b2c "monitor: add object-add (QMP) and > object_add (HMP) command". Parameter 'id' is the QemuOpts ID, thus > bound by its well-formedness rule. > > Therefore, -object and HMP object-add only support a subset of the > possible names. > > In particular, they do not permit "automatic arrayification". > > Should QOM names be (well-formed!) IDs? Yes, I think that is sane. Are there any invalid IDs used as QOM names today? Hopefully the answer is no and we can lock everything down using id_wellformed(). pgpKJl35Tib7e.pgp Description: PGP signature
Re: [Qemu-devel] IDs in QOM
Am 02.10.2014 um 15:21 schrieb Stefan Hajnoczi: > On Wed, Oct 01, 2014 at 02:33:47PM +0200, Markus Armbruster wrote: >> Markus Armbruster writes: > > This discussion seems orthogonal to your patch. But I'm not applying it > yet to give more time for discussion/review of the patch. > >> Is mangling array-ness into the name really a good idea? Isn't this >> type matter, not name matter? > > I agree. It's nasty to hack the array selector into the name and will > probably cause us pain down the line. > >> Backtracking a bit... Unlike QMP object-add, -object ) and HMP >> object-add use QemuOpts. See object_create(), commit 68d98d3 "vl: add >> -object option to create QOM objects from the command line", and >> hmp_object_add(), commit cff8b2c "monitor: add object-add (QMP) and >> object_add (HMP) command". Parameter 'id' is the QemuOpts ID, thus >> bound by its well-formedness rule. >> >> Therefore, -object and HMP object-add only support a subset of the >> possible names. >> >> In particular, they do not permit "automatic arrayification". >> >> Should QOM names be (well-formed!) IDs? > > Yes, I think that is sane. > > Are there any invalid IDs used as QOM names today? > > Hopefully the answer is no and we can lock everything down using > id_wellformed(). On IRC I was arguing against that, preferring some more specific object_property_name_wellformed() or so. This could be called from object_property_add(), with invalid names returning an Error *. Only thing to check for would be '/'? Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
On Thu, Oct 02, 2014 at 04:14:05PM +0300, Gal Hammer wrote: > On 02/10/2014 15:49, Michael S. Tsirkin wrote: > >On Wed, Sep 17, 2014 at 02:39:52PM +0300, Gal Hammer wrote: > >>Based on Microsoft's sepecifications (paper can be dowloaded from > >>http://go.microsoft.com/fwlink/?LinkId=260709), add a device > >>description to the SSDT ACPI table. > >> > >>The GUID is set using a new "vmgenid" device. > >> > >>Signed-off-by: Gal Hammer > >>Reviewed-By: Igor Mammedov > >>--- > >> default-configs/i386-softmmu.mak | 1 + > >> default-configs/x86_64-softmmu.mak | 1 + > >> hw/i386/Makefile.objs | 2 +- > >> hw/i386/acpi-build.c | 39 +++ > >> hw/i386/ssdt-vmgenid.dsl | 63 > >> hw/i386/ssdt-vmgenid.hex.generated | 206 > >> + > >> hw/misc/Makefile.objs | 1 + > >> hw/misc/vmgenid.c | 84 +++ > >> include/hw/i386/pc.h | 3 + > > > >Patch scripts/update-acpi.sh as well please. > > I understand what should be changed there. The script just copy all the > *.hex files to *.hex.generated. > > >> 9 files changed, 399 insertions(+), 1 deletion(-) > >> create mode 100644 hw/i386/ssdt-vmgenid.dsl > >> create mode 100644 hw/i386/ssdt-vmgenid.hex.generated > >> create mode 100644 hw/misc/vmgenid.c > >> > >>diff --git a/default-configs/i386-softmmu.mak > >>b/default-configs/i386-softmmu.mak > >>index 8e08841..bd33c75 100644 > >>--- a/default-configs/i386-softmmu.mak > >>+++ b/default-configs/i386-softmmu.mak > >>@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y > >> CONFIG_ICC_BUS=y > >> CONFIG_PVPANIC=y > >> CONFIG_MEM_HOTPLUG=y > >>+CONFIG_VMGENID=y > >>diff --git a/default-configs/x86_64-softmmu.mak > >>b/default-configs/x86_64-softmmu.mak > >>index 66557ac..006fc7c 100644 > >>--- a/default-configs/x86_64-softmmu.mak > >>+++ b/default-configs/x86_64-softmmu.mak > >>@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y > >> CONFIG_ICC_BUS=y > >> CONFIG_PVPANIC=y > >> CONFIG_MEM_HOTPLUG=y > >>+CONFIG_VMGENID=y > >>diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs > >>index 9d419ad..cd1beb3 100644 > >>--- a/hw/i386/Makefile.objs > >>+++ b/hw/i386/Makefile.objs > >>@@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c > >>hw/i386/acpi-dsdt.hex \ > >>hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \ > >>hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \ > >>hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \ > >>- hw/i386/ssdt-tpm.hex > >>+ hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex > >> > >> iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \ > >> ; then echo "$(2)"; else echo "$(3)"; fi ;) > >>diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >>index a313321..72d5a88 100644 > >>--- a/hw/i386/acpi-build.c > >>+++ b/hw/i386/acpi-build.c > >>@@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo { > >> const unsigned char *dsdt_code; > >> unsigned dsdt_size; > >> uint16_t pvpanic_port; > >>+bool vm_generation_id_set; > >>+uint8_t vm_generation_id[16]; > >> } AcpiMiscInfo; > >> > >> typedef struct AcpiBuildPciBusHotplugState { > >>@@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > >> info->has_hpet = hpet_find(); > >> info->has_tpm = tpm_find(); > >> info->pvpanic_port = pvpanic_port(); > >>+info->vm_generation_id_set = vm_generation_id(info->vm_generation_id); > >> } > >> > >> static void acpi_get_pci_info(PcPciInfo *info) > >>@@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val) > >> #include "hw/i386/ssdt-misc.hex" > >> #include "hw/i386/ssdt-pcihp.hex" > >> #include "hw/i386/ssdt-tpm.hex" > >>+#include "hw/i386/ssdt-vmgenid.hex" > >> > >> static void > >> build_append_notify_method(GArray *device, const char *name, > >>@@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker) > >> memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml)); > >> } > >> > >>+static void > >>+build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info) > >>+{ > >>+int vgid_start = table_data->len; > >>+void *vgid_ptr; > >>+uint8_t *vm_gid_ptr; > >>+uint32_t vm_gid_physical_address; > >>+ > >>+vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml)); > >>+memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml)); > >>+ > >>+vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml), > >>+ *ssdt_acpi_vm_gid, > >>+ sizeof(info->vm_generation_id)); > >>+memcpy(vm_gid_ptr, info->vm_generation_id, > >>+ sizeof(info->vm_generation_id)); > >>+ > >>+bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > >>+ ACPI_BUILD_TABLE_FILE, > >>+ table_data, > >>+ vgid_ptr + *ssdt_acpi_vm_gid_addr, > >>+
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
Il 02/10/2014 14:11, Michael S. Tsirkin ha scritto: > Summarizing what you say, there are two issues around ACPI tables: > - linuxboot uses FW CFG to for memory allocations, > seabios ignores that, so they might conflict. > Let's fix either linuxboot or seabios (or both!) > and forget about it. We can fix linuxboot, it is easy. These patches do fix John's scenario, but that is not the main issue. They are not an _attempt_ to fix it, they just do so more or less by chance. Their real purpose is fixing the second issue: > - table size changes cause cross version migration issues > this is really due to the fact we are using RAM > to migrate ACPI tables. > IMHO a more robust fix would be to allow RAM size to change > during migration, or to avoid using RAM, switch to another type of > object. Allowing fw_cfg size to change during migration (does not matter if it is stored in RAM or otherwise) is a huge can of worms because the host might have loaded the size and stored it somewhere, way before migration. Extreme example: the guest could expect the size to remain the same at boot time and S3 resume time. So I think the fw_cfg size is guest ABI and cannot change across migration anyway. > So both issues have other solutions, and I think it's a good > idea to focus on them for now. > Also, I really would like to avoid having ACPI sizing-related > issues for this release. The memory of 2.1.X pain is too fresh :) Yeah, I understand that. But I think the scary part of this series is actually the first two patches, rather than the ACPI sizing algorithm. Paolo > I'm not NACKing this patchset, but let's > make some progress on the bigger issues listed above, then come > back and address sizing as appropriate. > > Thanks! > >> >> Paolo Bonzini (6): >> pc: initialize fw_cfg earlier >> pc: load the kernel after ACPI tables are built >> pc: redo sizing of reserved high memory area for -kernel/-initrd >> pc: introduce new ACPI table sizing algorithm >> pc: go back to smaller ACPI tables >> pc: clean up pre-2.1 compatibility code >> >> hw/i386/acpi-build.c | 23 +--- >> hw/i386/pc.c | 72 >> +-- >> hw/i386/pc_piix.c| 32 ++ >> hw/i386/pc_q35.c |7 ++-- >> include/hw/i386/pc.h |4 ++ >> 5 files changed, 66 insertions(+), 72 deletions(-)
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
On Thu, Oct 02, 2014 at 03:30:57PM +0200, Paolo Bonzini wrote: > Il 02/10/2014 14:11, Michael S. Tsirkin ha scritto: > > Summarizing what you say, there are two issues around ACPI tables: > > - linuxboot uses FW CFG to for memory allocations, > > seabios ignores that, so they might conflict. > > Let's fix either linuxboot or seabios (or both!) > > and forget about it. > > We can fix linuxboot, it is easy. > > These patches do fix John's scenario, but that is not the main issue. > They are not an _attempt_ to fix it, they just do so more or less by > chance. Their real purpose is fixing the second issue: > > > - table size changes cause cross version migration issues > > this is really due to the fact we are using RAM > > to migrate ACPI tables. > > IMHO a more robust fix would be to allow RAM size to change > > during migration, or to avoid using RAM, switch to another type of > > object. > > Allowing fw_cfg size to change during migration (does not matter if it > is stored in RAM or otherwise) is a huge can of worms because the host > might have loaded the size and stored it somewhere, way before migration. Right. I'm not suggesting it. I suggest migrating fw cfg size instead. > Extreme example: the guest could expect the size to remain the same at > boot time and S3 resume time. AFAIK ACPI tables aren't re-read from QEMU at S3 resume. So guest will always see the same tables that are currently in RAM. > So I think the fw_cfg size is guest ABI and cannot change across > migration anyway. But this is already the case, this is not the issue. The issue is that incoming migration might have a different fw_cfg size from what we have. I think migrating this value will solve the issue in a cleaner way. > > > So both issues have other solutions, and I think it's a good > > idea to focus on them for now. > > Also, I really would like to avoid having ACPI sizing-related > > issues for this release. The memory of 2.1.X pain is too fresh :) > > Yeah, I understand that. But I think the scary part of this series is > actually the first two patches, rather than the ACPI sizing algorithm. > > Paolo > > > I'm not NACKing this patchset, but let's > > make some progress on the bigger issues listed above, then come > > back and address sizing as appropriate. > > > > Thanks! > > > >> > >> Paolo Bonzini (6): > >> pc: initialize fw_cfg earlier > >> pc: load the kernel after ACPI tables are built > >> pc: redo sizing of reserved high memory area for -kernel/-initrd > >> pc: introduce new ACPI table sizing algorithm > >> pc: go back to smaller ACPI tables > >> pc: clean up pre-2.1 compatibility code > >> > >> hw/i386/acpi-build.c | 23 +--- > >> hw/i386/pc.c | 72 > >> +-- > >> hw/i386/pc_piix.c| 32 ++ > >> hw/i386/pc_q35.c |7 ++-- > >> include/hw/i386/pc.h |4 ++ > >> 5 files changed, 66 insertions(+), 72 deletions(-)
Re: [Qemu-devel] [PATCH] util: Emancipate id_wellformed() from QemuOpts
Stefan Hajnoczi writes: > On Tue, Sep 30, 2014 at 01:59:30PM +0200, Markus Armbruster wrote: >> +bool id_wellformed(const char *id) >> +{ >> +int i; >> + >> +if (!qemu_isalpha(id[0])) { >> +return 0; > > false would be a bit nicer since the other return cases use true/false. Missed when I changed the value to bool. Would you be willing to fix this up on commit? If not, I'll respin.
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
Il 02/10/2014 15:41, Michael S. Tsirkin ha scritto: > On Thu, Oct 02, 2014 at 03:30:57PM +0200, Paolo Bonzini wrote: >> These patches do fix John's scenario, but that is not the main issue. >> They are not an _attempt_ to fix it, they just do so more or less by >> chance. Their real purpose is fixing the second issue: >> >>> - table size changes cause cross version migration issues >>> this is really due to the fact we are using RAM >>> to migrate ACPI tables. >>> IMHO a more robust fix would be to allow RAM size to change >>> during migration, or to avoid using RAM, switch to another type of >>> object. >> >> Allowing fw_cfg size to change during migration (does not matter if it >> is stored in RAM or otherwise) is a huge can of worms because the host >> might have loaded the size and stored it somewhere, way before migration. > > Right. I'm not suggesting it. I suggest migrating fw cfg size instead. > > The issue is that incoming migration might have a different > fw_cfg size from what we have. Understood now. > I think migrating this value will solve the issue in a cleaner way. Perhaps. The question is whether it would complicate the forwards-migration code beyond what is sane. I think we are practically speaking stuck with RAM. Paolo
Re: [Qemu-devel] [PATCH 0/6] pc: bring ACPI table size below to 2.0 levels, try fixing -initrd for good
On Thu, Oct 02, 2014 at 03:43:35PM +0200, Paolo Bonzini wrote: > Il 02/10/2014 15:41, Michael S. Tsirkin ha scritto: > > On Thu, Oct 02, 2014 at 03:30:57PM +0200, Paolo Bonzini wrote: > >> These patches do fix John's scenario, but that is not the main issue. > >> They are not an _attempt_ to fix it, they just do so more or less by > >> chance. Their real purpose is fixing the second issue: > >> > >>> - table size changes cause cross version migration issues > >>> this is really due to the fact we are using RAM > >>> to migrate ACPI tables. > >>> IMHO a more robust fix would be to allow RAM size to change > >>> during migration, or to avoid using RAM, switch to another type of > >>> object. > >> > >> Allowing fw_cfg size to change during migration (does not matter if it > >> is stored in RAM or otherwise) is a huge can of worms because the host > >> might have loaded the size and stored it somewhere, way before migration. > > > > Right. I'm not suggesting it. I suggest migrating fw cfg size instead. > > > > The issue is that incoming migration might have a different > > fw_cfg size from what we have. > > Understood now. > > > I think migrating this value will solve the issue in a cleaner way. > > Perhaps. The question is whether it would complicate the > forwards-migration code beyond what is sane. I think we are practically > speaking stuck with RAM. > > Paolo Migrating RAM size is actually useful too, I think someone asked for it. -- MST
Re: [Qemu-devel] [PATCH 2/2] i386: Add a Virtual Machine Generation ID device.
On 02/10/2014 15:49, Michael S. Tsirkin wrote: On Wed, Sep 17, 2014 at 02:39:52PM +0300, Gal Hammer wrote: Based on Microsoft's sepecifications (paper can be dowloaded from http://go.microsoft.com/fwlink/?LinkId=260709), add a device description to the SSDT ACPI table. The GUID is set using a new "vmgenid" device. Signed-off-by: Gal Hammer Reviewed-By: Igor Mammedov --- default-configs/i386-softmmu.mak | 1 + default-configs/x86_64-softmmu.mak | 1 + hw/i386/Makefile.objs | 2 +- hw/i386/acpi-build.c | 39 +++ hw/i386/ssdt-vmgenid.dsl | 63 hw/i386/ssdt-vmgenid.hex.generated | 206 + hw/misc/Makefile.objs | 1 + hw/misc/vmgenid.c | 84 +++ include/hw/i386/pc.h | 3 + Patch scripts/update-acpi.sh as well please. I understand what should be changed there. The script just copy all the *.hex files to *.hex.generated. 9 files changed, 399 insertions(+), 1 deletion(-) create mode 100644 hw/i386/ssdt-vmgenid.dsl create mode 100644 hw/i386/ssdt-vmgenid.hex.generated create mode 100644 hw/misc/vmgenid.c diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 8e08841..bd33c75 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y CONFIG_ICC_BUS=y CONFIG_PVPANIC=y CONFIG_MEM_HOTPLUG=y +CONFIG_VMGENID=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 66557ac..006fc7c 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y CONFIG_ICC_BUS=y CONFIG_PVPANIC=y CONFIG_MEM_HOTPLUG=y +CONFIG_VMGENID=y diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs index 9d419ad..cd1beb3 100644 --- a/hw/i386/Makefile.objs +++ b/hw/i386/Makefile.objs @@ -12,7 +12,7 @@ hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \ hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \ hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \ hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex \ - hw/i386/ssdt-tpm.hex + hw/i386/ssdt-tpm.hex hw/i386/ssdt-vmgenid.hex iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \ ; then echo "$(2)"; else echo "$(3)"; fi ;) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index a313321..72d5a88 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -96,6 +96,8 @@ typedef struct AcpiMiscInfo { const unsigned char *dsdt_code; unsigned dsdt_size; uint16_t pvpanic_port; +bool vm_generation_id_set; +uint8_t vm_generation_id[16]; } AcpiMiscInfo; typedef struct AcpiBuildPciBusHotplugState { @@ -216,6 +218,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) info->has_hpet = hpet_find(); info->has_tpm = tpm_find(); info->pvpanic_port = pvpanic_port(); +info->vm_generation_id_set = vm_generation_id(info->vm_generation_id); } static void acpi_get_pci_info(PcPciInfo *info) @@ -710,6 +713,7 @@ static inline char acpi_get_hex(uint32_t val) #include "hw/i386/ssdt-misc.hex" #include "hw/i386/ssdt-pcihp.hex" #include "hw/i386/ssdt-tpm.hex" +#include "hw/i386/ssdt-vmgenid.hex" static void build_append_notify_method(GArray *device, const char *name, @@ -1246,6 +1250,37 @@ build_tpm_ssdt(GArray *table_data, GArray *linker) memcpy(tpm_ptr, ssdt_tpm_aml, sizeof(ssdt_tpm_aml)); } +static void +build_vmgenid_ssdt(GArray *table_data, GArray *linker, AcpiMiscInfo *info) +{ +int vgid_start = table_data->len; +void *vgid_ptr; +uint8_t *vm_gid_ptr; +uint32_t vm_gid_physical_address; + +vgid_ptr = acpi_data_push(table_data, sizeof(ssdt_vmgenid_aml)); +memcpy(vgid_ptr, ssdt_vmgenid_aml, sizeof(ssdt_vmgenid_aml)); + +vm_gid_ptr = acpi_data_get_ptr(vgid_ptr, sizeof(ssdt_vmgenid_aml), + *ssdt_acpi_vm_gid, + sizeof(info->vm_generation_id)); +memcpy(vm_gid_ptr, info->vm_generation_id, + sizeof(info->vm_generation_id)); + +bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, + ACPI_BUILD_TABLE_FILE, + table_data, + vgid_ptr + *ssdt_acpi_vm_gid_addr, + sizeof(uint32_t)); + +vm_gid_physical_address = vgid_start + *ssdt_acpi_vm_gid; +ACPI_BUILD_SET_LE(vgid_ptr, sizeof(ssdt_vmgenid_aml), + *ssdt_acpi_vm_gid_addr, 32, vm_gid_physical_address); + +build_header(linker, table_data, vgid_ptr, "SSDT", + sizeof(ssdt_vmgenid_aml), 1); +} + typedef enum { MEM_AFFINITY_NOFLAGS = 0, MEM_AFFINITY_ENABLED = (1 << 0), @@ -1617,6 +1652,10 @@ void acpi_build(PcGuestI