Re: [Qemu-devel] migration: qemu-coroutine-lock.c:141: qemu_co_mutex_unlock: Assertion `mutex->locked == 1' failed
On 09/16/2014 10:34 PM, Kevin Wolf wrote:
> Am 16.09.2014 um 14:10 hat Paolo Bonzini geschrieben:
>> Il 16/09/2014 14:02, Alexey Kardashevskiy ha scritto:
>>> I am having problems when migrate a guest via libvirt like this:
>>>
>>> virsh migrate --live --persistent --undefinesource --copy-storage-all
>>> --verbose --desturi qemu+ssh://legkvm/system --domain chig1
>>>
>>> The XML used to create the guest is at the end of this mail.
>>>
>>> I see NBD FLUSH command after the destination QEMU received EOF for
>>> migration stream and this produces a crash in qcow2_co_flush_to_os() as
>>> s->lock is false or s->l2_table_cache is NULL.
>>>
>>
>> Max, Kevin, could the fix be something like this?
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 0daf25c..e7459ea 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -1442,6 +1442,7 @@ static void qcow2_invalidate_cache(BlockDriverState
>> *bs, Error **errp)
>> memcpy(&aes_decrypt_key, &s->aes_decrypt_key,
>> sizeof(aes_decrypt_key));
>> }
>>
>> +qemu_co_mutex_lock(&s->lock);
>> qcow2_close(bs);
>>
>> bdrv_invalidate_cache(bs->file, &local_err);
>> @@ -1455,6 +1456,7 @@ static void qcow2_invalidate_cache(BlockDriverState
>> *bs, Error **errp)
>>
>> ret = qcow2_open(bs, options, flags, &local_err);
>> QDECREF(options);
>> +qemu_co_mutex_unlock(&s->lock);
>> if (local_err) {
>> error_setg(errp, "Could not reopen qcow2 layer: %s",
>> error_get_pretty(local_err));
>>
>> On top of this, *_invalidate_cache needs to be marked as coroutine_fn.
>
> I think bdrv_invalidate_cache() really needs to call bdrv_drain_all()
> before starting to reopen stuff. There could be requests in flight
> without holding the lock and if you can indeed reopen their BDS under
> their feet without breaking things (I doubt it), that would be pure
> luck.
I tried the patch below and it did not help. So I assume I did it wrong,
could you please explain more? Thanks!
diff --git a/block.c b/block.c
index 2df600e..ecc876d 100644
--- a/block.c
+++ b/block.c
@@ -5038,11 +5038,16 @@ void bdrv_invalidate_cache(BlockDriverState *bs,
Error **errp)
return;
}
+bdrv_drain_all();
+
if (bs->drv->bdrv_invalidate_cache) {
bs->drv->bdrv_invalidate_cache(bs, &local_err);
} else if (bs->file) {
bdrv_invalidate_cache(bs->file, &local_err);
}
+
+bdrv_drain_all();
+
if (local_err) {
error_propagate(errp, local_err);
return;
--
Alexey
Re: [Qemu-devel] [PATCH] block: vhdx - fix reading beyond pointer during image creation
Jeff Cody writes:
> In vhdx_create_metadata(), we allocate 40 bytes to entry_buffer for
> the various metadata table entries. However, we write out 64kB from
> that buffer into the new file. Only write out the correct 40 bytes.
>
> Signed-off-by: Jeff Cody
> ---
> block/vhdx.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 796b7bd..b52ec32 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1407,6 +1407,12 @@ exit:
> return ret;
> }
>
> +#define VHDX_METADATA_ENTRY_BUFFER_SIZE \
> +(sizeof(VHDXFileParameters)
> +\
> + sizeof(VHDXVirtualDiskSize)
> +\
> + sizeof(VHDXPage83Data)
> +\
> +
> sizeof(VHDXVirtualDiskLogicalSectorSize) +\
> +
> sizeof(VHDXVirtualDiskPhysicalSectorSize))
Long lines, caused by excessive indentation. Emacs suggests
#define VHDX_METADATA_ENTRY_BUFFER_SIZE \
(sizeof(VHDXFileParameters) + \
sizeof(VHDXVirtualDiskSize) + \
sizeof(VHDXPage83Data) + \
sizeof(VHDXVirtualDiskLogicalSectorSize) + \
sizeof(VHDXVirtualDiskPhysicalSectorSize))
>
> /*
> * Create the Metadata entries.
> @@ -1445,11 +1451,7 @@ static int vhdx_create_new_metadata(BlockDriverState
> *bs,
> VHDXVirtualDiskLogicalSectorSize *mt_log_sector_size;
> VHDXVirtualDiskPhysicalSectorSize *mt_phys_sector_size;
>
> -entry_buffer = g_malloc0(sizeof(VHDXFileParameters) +
> - sizeof(VHDXVirtualDiskSize) +
> - sizeof(VHDXPage83Data) +
> - sizeof(VHDXVirtualDiskLogicalSectorSize) +
> - sizeof(VHDXVirtualDiskPhysicalSectorSize));
> +entry_buffer = g_malloc0(VHDX_METADATA_ENTRY_BUFFER_SIZE);
>
> mt_file_params = entry_buffer;
> offset += sizeof(VHDXFileParameters);
> @@ -1530,7 +1532,7 @@ static int vhdx_create_new_metadata(BlockDriverState
> *bs,
> }
>
> ret = bdrv_pwrite(bs, metadata_offset + (64 * KiB), entry_buffer,
> - VHDX_HEADER_BLOCK_SIZE);
> + VHDX_METADATA_ENTRY_BUFFER_SIZE);
> if (ret < 0) {
> goto exit;
> }
Fixes read beyond end of buffer. Crash bug when sufficiently unlucky.
> @@ -1725,7 +1727,6 @@ static int
> vhdx_create_new_region_table(BlockDriverState *bs,
> goto exit;
> }
>
> -
> exit:
> g_free(s);
> g_free(buffer);
> @@ -1876,7 +1877,6 @@ static int vhdx_create(const char *filename, QemuOpts
> *opts, Error **errp)
> }
>
>
> -
> delete_and_exit:
> bdrv_unref(bs);
> exit:
These two hunks are unrelated. I wouldn't include them. Advice, not
objection.
Keeping two out of three blank lines looks odd, but pairs of blank lines
occur elsewhere in the function, so I guess it's intentional.
If you clean up the long lines, you can add my R-by.
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
> What if instead of having a "mmu_index" for the mmu arrays, we have a pointer > to the "mmu context". This does imply an extra memory load on the fast path, > but probably not an extra instruction. > > With this, we can suddenly afford to have a relatively large number of mmu > contexts, with which we could implement address space numbers for relevant > targets. > > It is, of course, a much larger change, but perhaps it's of larger benefit. Sounds good. I can give it a shot---in the meanwhile, since I forgot to Cc qemu-ppc, Alex can you review/apply patch 1? Paolo
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
"Michael S. Tsirkin" writes: > On Tue, Sep 16, 2014 at 10:01:19PM +0300, Michael S. Tsirkin wrote: >> On Tue, Sep 16, 2014 at 08:31:08PM +0200, Paolo Bonzini wrote: >> > Il 16/09/2014 18:56, Michael S. Tsirkin ha scritto: >> > > On Tue, Sep 16, 2014 at 06:27:51PM +0200, Paolo Bonzini wrote: >> > >> Il 16/09/2014 18:26, Michael S. Tsirkin ha scritto: >> > >>> Right so types should be explicit. >> > >>> If an arbitrary string isn't allowed, this should be documented. >> > >>> It's not great as is: what's the format for macaddr? AA:BB:CC:DD:EE:FF? >> > >>> aa:bb:cc:dd:ee:ff? aabbccddeeff? 0xaabbccddeeff? >> > >>> But just saying "string" is going in the wrong direction imho. >> > >> >> > >> That's the purpose of documentation (docs/qdev-device-use.txt), >> > > >> > > That's not user documentation, that's developer documentation, >> > > isn't it? >> > >> > It's user documentation. It's not distributed because we suck at >> > documentation. >> > >> > >> and even >> > >> then is better done with examples. I don't think doing it in -device >> > >> foo,help (which I'm not even sure is particularly helpful. >> > > >> > > -device foo,help isn't helpful at all because it does not >> > > tell people what does each option do. >> > > But it really should be fixed. >> > >> > Exactly. >> > >> > >> I'm sympathetic towards fixing the drive->str change, but I have no idea >> > >> how to do it. >> > > >> > > Change legacy_name to point to a detailed human-readable >> > > description of the type? >> > > E.g. "Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF"? >> > >> > If libvirt can cope with >> > >> > e1000.mac=str (Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF) >> > >> > that would work for me. >> >> Shouldn't "str" be "string" in HMP? Probably. What about QMP? JSON calls the type "string". Possible justification for "str": QMP makes up its own type system, losely based on JSON's, with its own terminology. I'm not sure "=T" adds value over the description. Certainly not in the example above. >> Eblake - type is ignored right? Does this mean anything to the right of >> = is ignored? As far as I can tell from the libvirt sources: yes. >> > > We really really should add description to all properties, too. >> > >> > This is a huge job. We have hundreds of properties. >> > >> > Paolo >> >> Right. If we don't start we won't get there though, will we? >> > > And, we'll keep adding undocumented ones. Agree with all three points. We should get our act together on property documentation.
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Eric Blake writes: > On 09/16/2014 12:31 PM, Paolo Bonzini wrote: > >>> Change legacy_name to point to a detailed human-readable >>> description of the type? >>> E.g. "Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF"? >> >> If libvirt can cope with >> >> e1000.mac=str (Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF) > > Isn't this output only available under -help? Libvirt only cares about > QMP listings of devices and their properties, so improving the human > interface should have no negative impact to the machine interface. Libvirt still has code parsing this, see virQEMUCapsExtractDeviceStr(). Whether it's actually used with recent QEMU I don't know. -device FOO,help merely formats qmp_device_list_properties()'s value for human consumption. So, to make a description string available for help, we also have to put it into the function's value. If we don't want it in QMP, we need to filter it out in the command handler. Right now, qmp_device_list_properties() *is* the command handler. Aside: why isn't the QMP command named query-device-list-properties? We suck at consistency!
[Qemu-devel] vhost-user:is miss command VHOST_NET_SET_BACKEND?
Hi, The VHOST_NET_SET_BACKEND could tell the user when the backend is created or destroyed.is usefull for the user but this command is lost in the protocol
[Qemu-devel] vhost-user:why region[0] always mmap failed ?
Hi,
There is two memory regions when receive VHOST_SET_MEM_TABLE message:
region[0]
gpa = 0x0
size = 655360
ua = 0x2ac0
offset = 0
region[1]
gpa = 0xC
size = 2146697216
ua = 0x2acc
offset = 786432
region[0] always mmap failed.The user code is :
for (idx = 0; idx < msg->msg.memory.nregions; idx++) {
if (msg->fds[idx] > 0) {
size_t size;
uint64_t *guest_mem;
Region *region = &vhost_server->memory.regions[i];
region->guest_phys_addr =
msg->msg.memory.regions[idx].guest_phys_addr;
region->memory_size = msg->msg.memory.regions[idx].memory_size;
region->userspace_addr =
msg->msg.memory.regions[idx].userspace_addr;
region->mmap_offset = msg->msg.memory.regions[idx].mmap_offset;
assert(idx < msg->fd_num);
assert(msg->fds[idx] > 0);
size = region->memory_size + region->mmap_offset;
guest_mem = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED,
msg->fds[idx], 0);
if (MAP_FAILED == guest_mem) {
continue;
}
i++;
guest_mem += (region->mmap_offset / sizeof(*guest_mem));
region->mmap_addr = (uint64_t)guest_mem;
vhost_server->memory.nregions++;
}
}
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: Tuesday, September 16, 2014 10:37 PM > Subject: Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties > > Il 16/09/2014 16:32, Markus Armbruster ha scritto: > > Paolo Bonzini writes: > > > >> Il 16/09/2014 11:16, Markus Armbruster ha scritto: > I think both "str" and "link" actually are a small > degradation > compared to "drive", and this is why I kept the legacy_name. But > overall I > think it's not really worth the layering violation that patches 2 and 3 > are; > and it's definitely not stable material. > >>> > >>> "str" is clearly a degradation for me. I breaks usage like > >>> > >>> for i in `qemu -device help 2>&1 | sed -n 's/^name > >>> "\([^"]*\)".*/\1/p'` > >>> do qemu -device $i,help 2>&1 > >>> done | grep =drive > >>> > >>> Finds all block device models. I've done such things many times. > >> > >> Just replace "grep =drive" with "fgrep .drive". Similarly: > >> > >> =chr -> .chardev (bonus: matches the command line option) > >> =netdev -> .netdev > >> =vlan-> .vlan > >> =macaddr -> .mac > > > > Unlike =drive, this isn't guaranteed to work. > > If it doesn't, when we've been consistent so far, it's a bug. > > >> It is, but we're suprisingly consistent in the naming of such > >> special-typed properties. So it's actually a good thing that > >> legacy_name provides redundant information. > > > > Given our overall consistency track record: yes, it's surprising, and no, > > I'm not comfortable relying on us being consistent :) > > The point of stuff like QOM is exactly to force us to be consistent. > No, it doesn't always work. But patches 2 and 3 really are a layering > violation. I have no idea how to bring back "drive" without introducing > a layering violation somewhere, but this one is really too much... > Sorry, I can't understand the layering violation on PATCH2 and PATCH3. AliasProperty struct is QOM object and ObjectProperty is also QOM object, I just move AliasProperty struct from Object.c to Object.h meanwhile add a point reference in ObjectProperty struct for AliasProperty. Does this is a layering violation? Hope for your more detailed information about this, thanks in advance! :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH] block: delete cow block driver
On Tue, 09/16 15:24, Stefan Hajnoczi wrote:
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index a685d02..56302c7 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -197,7 +197,7 @@
> # 'file', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
> # 'host_floppy', 'http', 'https', 'nbd', 'parallels', 'qcow',
> # 'qcow2', 'raw', 'tftp', 'vdi', 'vmdk', 'vpc', 'vvfat'
> -# 2.2: 'archipelago'
> +# 2.2: 'archipelago' added, 'cow' dropped
> #
> # @backing_file: #optional the name of the backing file (for copy-on-write)
> #
> @@ -1149,7 +1149,7 @@
> { 'enum': 'BlockdevDriver',
>'data': [ 'archipelago', 'file', 'host_device', 'host_cdrom',
> 'host_floppy',
> 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdebug',
> -'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels', 'qcow',
> +'blkverify', 'bochs', 'cloop', 'dmg', 'parallels', 'qcow',
> 'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum' ] }
>
> ##
> @@ -1491,7 +1491,6 @@
>'blkverify': 'BlockdevOptionsBlkverify',
>'bochs': 'BlockdevOptionsGenericFormat',
>'cloop': 'BlockdevOptionsGenericFormat',
> - 'cow':'BlockdevOptionsGenericCOWFormat',
>'dmg':'BlockdevOptionsGenericFormat',
>'parallels': 'BlockdevOptionsGenericFormat',
>'qcow': 'BlockdevOptionsGenericCOWFormat',
Have a conflict with "[PATCH v7 0/3] block: Introduce "null" drivers" which you
applied, where these lists are sorted alphabetically.
Otherwise looks good to me.
Thanks,
Fam
[Qemu-devel] [PATCH V3 7/7] acpi/cpu-hotplug: introduce help function to keep bit setting in one place
Introduce help function acpi_set_local_sts() to simplify acpi_cpu_plug_cb
and acpi_cpu_hotplug_init, so that we can keep bit setting in one place.
Signed-off-by: Gu Zheng
---
hw/acpi/cpu_hotplug.c | 22 +++---
1 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 7629686..d42c961 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -36,8 +36,8 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
},
};
-void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
- AcpiCpuHotplug *g, CPUState *cpu, Error **errp)
+static void acpi_set_local_sts(AcpiCpuHotplug *g, CPUState *cpu,
+ Error **errp)
{
CPUClass *k = CPU_GET_CLASS(cpu);
int64_t cpu_id;
@@ -48,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
return;
}
-ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
+}
+void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+ AcpiCpuHotplug *g, CPUState *cpu, Error **errp)
+{
+acpi_set_local_sts(g, cpu, errp);
+if (*errp != NULL) {
+return;
+}
+
+ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
acpi_update_sci(ar, irq);
}
@@ -60,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object
*owner,
CPUState *cpu;
CPU_FOREACH(cpu) {
-CPUClass *cc = CPU_GET_CLASS(cpu);
-int64_t id = cc->get_arch_id(cpu);
+Error *local_err = NULL;
-g_assert((id / 8) < ACPI_GPE_PROC_LEN);
-gpe_cpu->sts[id / 8] |= (1 << (id % 8));
+acpi_set_local_sts(gpe_cpu, cpu, &local_err);
+g_assert(local_err == NULL);
}
memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
--
1.7.7
[Qemu-devel] [PATCH V3 5/7] pc: Update rtc_cmos in pc_cpu_plug
Update rtc_cmos in pc_cpu_plug directly instead of the notifier, with
this change, there will no user of CPU hot-plug notifier any more, so
remove it.
Signed-off-by: Gu Zheng
---
hw/i386/pc.c| 25 ++---
include/sysemu/sysemu.h |3 ---
qom/cpu.c | 10 --
3 files changed, 6 insertions(+), 32 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e25e71b..8b887c7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -353,20 +353,7 @@ static void pc_cmos_init_late(void *opaque)
qemu_unregister_reset(pc_cmos_init_late, opaque);
}
-typedef struct RTCCPUHotplugArg {
-Notifier cpu_added_notifier;
-ISADevice *rtc_state;
-} RTCCPUHotplugArg;
-
-static void rtc_notify_cpu_added(Notifier *notifier, void *data)
-{
-RTCCPUHotplugArg *arg = container_of(notifier, RTCCPUHotplugArg,
- cpu_added_notifier);
-ISADevice *s = arg->rtc_state;
-
-/* increment the number of CPUs */
-rtc_set_memory(s, 0x5f, rtc_get_memory(s, 0x5f) + 1);
-}
+static ISADevice *rtc_state;
void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
const char *boot_device,
@@ -376,7 +363,6 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
int val, nb, i;
FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
static pc_cmos_init_late_arg arg;
-static RTCCPUHotplugArg cpu_hotplug_cb;
/* various important CMOS locations needed by PC/Bochs bios */
@@ -415,10 +401,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
/* set the number of CPU */
rtc_set_memory(s, 0x5f, smp_cpus - 1);
-/* init CPU hotplug notifier */
-cpu_hotplug_cb.rtc_state = s;
-cpu_hotplug_cb.cpu_added_notifier.notify = rtc_notify_cpu_added;
-qemu_register_cpu_added_notifier(&cpu_hotplug_cb.cpu_added_notifier);
+
+rtc_state = s;
if (set_boot_dev(s, boot_device)) {
exit(1);
@@ -1630,6 +1614,9 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+
+/* increment the number of CPUs */
+rtc_set_memory(rtc_state, 0x5f, rtc_get_memory(rtc_state, 0x5f) + 1);
out:
error_propagate(errp, local_err);
}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..acfe494 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -183,9 +183,6 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict
*qdict);
/* generic hotplug */
void drive_hot_add(Monitor *mon, const QDict *qdict);
-/* CPU hotplug */
-void qemu_register_cpu_added_notifier(Notifier *notifier);
-
/* pcie aer error injection */
void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
int do_pcie_aer_inject_error(Monitor *mon,
diff --git a/qom/cpu.c b/qom/cpu.c
index b32dd0a..19c5de5 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -107,15 +107,6 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
}
-/* CPU hot-plug notifiers */
-static NotifierList cpu_added_notifiers =
-NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
-
-void qemu_register_cpu_added_notifier(Notifier *notifier)
-{
-notifier_list_add(&cpu_added_notifiers, notifier);
-}
-
void cpu_reset_interrupt(CPUState *cpu, int mask)
{
cpu->interrupt_request &= ~mask;
@@ -303,7 +294,6 @@ static void cpu_common_realizefn(DeviceState *dev, Error
**errp)
if (dev->hotplugged) {
cpu_synchronize_post_init(cpu);
-notifier_list_notify(&cpu_added_notifiers, dev);
cpu_resume(cpu);
}
}
--
1.7.7
[Qemu-devel] [PATCH V3 6/7] cpu-hotplug: rename function for better readability
Rename:
AcpiCpuHotplug_init --> acpi_cpu_hotplug_init
AcpiCpuHotplug_ops --> acpi_cpu_hotplug_ops
for better readability, just cleanup.
Signed-off-by: Gu Zheng
---
hw/acpi/cpu_hotplug.c |4 ++--
hw/acpi/ich9.c|4 ++--
hw/acpi/piix4.c |4 ++--
include/hw/acpi/cpu_hotplug.h |4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 0b9fa55..7629686 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -54,8 +54,8 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
acpi_update_sci(ar, irq);
}
-void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
- AcpiCpuHotplug *gpe_cpu, uint16_t base)
+void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
+ AcpiCpuHotplug *gpe_cpu, uint16_t base)
{
CPUState *cpu;
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index c53d4ab..42cf8fa 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -235,8 +235,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
pm->powerdown_notifier.notify = pm_powerdown_req;
qemu_register_powerdown_notifier(&pm->powerdown_notifier);
-AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
-&pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
+acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
+ &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
if (pm->acpi_memory_hotplug.is_enabled) {
acpi_memory_hotplug_init(pci_address_space_io(lpc_pci),
OBJECT(lpc_pci),
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b046cd3..205ba66 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -555,8 +555,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion
*parent,
acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent,
s->use_acpi_pci_hotplug);
-AcpiCpuHotplug_init(parent, OBJECT(s), &s->gpe_cpu,
-PIIX4_CPU_HOTPLUG_IO_BASE);
+acpi_cpu_hotplug_init(parent, OBJECT(s), &s->gpe_cpu,
+ PIIX4_CPU_HOTPLUG_IO_BASE);
if (s->acpi_memory_hotplug.is_enabled) {
acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 8188630..d015bef 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -23,6 +23,6 @@ typedef struct AcpiCpuHotplug {
void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
AcpiCpuHotplug *g, CPUState *dev, Error **errp);
-void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
- AcpiCpuHotplug *gpe_cpu, uint16_t base);
+void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
+ AcpiCpuHotplug *gpe_cpu, uint16_t base);
#endif
--
1.7.7
Re: [Qemu-devel] [RFC PATCH 07/17] COLO buffer: implement colo buffer as well as QEMUFileOps based on it
Hi 在 08/01/2014 10:52 PM, Dr. David Alan Gilbert 写道: * Yang Hongyang (yan...@cn.fujitsu.com) wrote: We need a buffer to store migration data. On save side: all saved data was write into colo buffer first, so that we can know the total size of the migration data. this can also separate the data transmission from colo control data, we use colo control data over socket fd to synchronous both side's stat. On restore side: all migration data was read into colo buffer first, then load data from the buffer: If network error happens while data transmission, the slaver can still functinal because the migration data are not yet loaded. This is very similar to the QEMUSizedBuffer based QEMUFile's that Stefan Berger wrote and that I use in both my postcopy and BER patchsets: http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg00846.html (and to the similar code from Isaku Yamahata). I think we should be able to use a shared version even if we need some changes. I've being using this patch as COLO buffer, see my latest branch: https://github.com/macrosheep/qemu/tree/colo_v0.4 But this QEMUSizedBuffer does not exactly match our needs, although it can work. We need a static buffer that lives through COLO process so that we don't need to alloc/free buffers every checkpoint. Currently open the buffer for write always alloc a new buffer, I think I need the following modification: 1. ability to open an existing QEMUSizedBuffer for write 2. a reset_buf() api, that will clear buffered data, or just rewind QEMUFile related to the buffer is enough. Signed-off-by: Yang Hongyang --- migration-colo.c | 112 +++ 1 file changed, 112 insertions(+) diff --git a/migration-colo.c b/migration-colo.c index d566b9d..b90d9b6 100644 --- a/migration-colo.c +++ b/migration-colo.c @@ -11,6 +11,7 @@ #include "qemu/main-loop.h" #include "qemu/thread.h" #include "block/coroutine.h" +#include "qemu/error-report.h" #include "migration/migration-colo.h" static QEMUBH *colo_bh; @@ -20,14 +21,122 @@ bool colo_supported(void) return true; } +/* colo buffer */ + +#define COLO_BUFFER_BASE_SIZE (1000*1000*4ULL) +#define COLO_BUFFER_MAX_SIZE (1000*1000*1000*10ULL) Powers of 2 are nicer! Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK . -- Thanks, Yang.
[Qemu-devel] [PATCH V3 4/7] pc: add cpu hotplug handler to PC_MACHINE
Add cpu hotplug handler to PC_MACHINE, which will perform the acpi
cpu hotplug callback via hotplug_handler API.
v3:
-deal with start up cpus in a more neat way as Igor suggested.
v2:
-just rebase.
Signed-off-by: Gu Zheng
---
hw/i386/pc.c | 26 +-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b6c9b61..e25e71b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1613,11 +1613,34 @@ out:
error_propagate(errp, local_err);
}
+static void pc_cpu_plug(HotplugHandler *hotplug_dev,
+DeviceState *dev, Error **errp)
+{
+HotplugHandlerClass *hhc;
+Error *local_err = NULL;
+PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+
+if (!pcms->acpi_dev) {
+if (dev->hotplugged) {
+error_setg(&local_err,
+ "cpu hotplug is not enabled: missing acpi device");
+}
+goto out;
+}
+
+hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
+hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+out:
+error_propagate(errp, local_err);
+}
+
static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
pc_dimm_plug(hotplug_dev, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+pc_cpu_plug(hotplug_dev, dev, errp);
}
}
@@ -1626,7 +1649,8 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState
*machine,
{
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
-if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
+object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
return HOTPLUG_HANDLER(machine);
}
--
1.7.7
[Qemu-devel] [PATCH V3 3/7] acpi:piix4: convert cpu hotplug handle to hotplug_handler API
Convert notifier based hotplug handle to hotplug_handler API,
and remove the unused AcpiCpuHotplug_add().
v2:
-remove the unused AcpiCpuHotplug_add().
Signed-off-by: Gu Zheng
---
hw/acpi/cpu_hotplug.c | 14 ++
hw/acpi/piix4.c | 14 ++
include/hw/acpi/cpu_hotplug.h |2 --
3 files changed, 4 insertions(+), 26 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index dfd6de5..0b9fa55 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -48,22 +48,12 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
return;
}
-AcpiCpuHotplug_add(&ar->gpe, g, cpu);
+ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
+g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
acpi_update_sci(ar, irq);
}
-void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu)
-{
-CPUClass *k = CPU_GET_CLASS(cpu);
-int64_t cpu_id;
-
-*gpe->sts = *gpe->sts | ACPI_CPU_HOTPLUG_STATUS;
-cpu_id = k->get_arch_id(CPU(cpu));
-g_assert((cpu_id / 8) < ACPI_GPE_PROC_LEN);
-g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
-}
-
void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
AcpiCpuHotplug *gpe_cpu, uint16_t base)
{
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..b046cd3 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -83,7 +83,6 @@ typedef struct PIIX4PMState {
uint8_t s4_val;
AcpiCpuHotplug gpe_cpu;
-Notifier cpu_added_notifier;
MemHotplugState acpi_memory_hotplug;
} PIIX4PMState;
@@ -348,6 +347,8 @@ static void piix4_device_plug_cb(HotplugHandler
*hotplug_dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+acpi_cpu_plug_cb(&s->ar, s->irq, &s->gpe_cpu, CPU(dev), errp);
} else {
error_setg(errp, "acpi: device plug request for not supported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -544,15 +545,6 @@ static const MemoryRegionOps piix4_gpe_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static void piix4_cpu_added_req(Notifier *n, void *opaque)
-{
-PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier);
-
-assert(s != NULL);
-AcpiCpuHotplug_add(&s->ar.gpe, &s->gpe_cpu, CPU(opaque));
-acpi_update_sci(&s->ar, s->irq);
-}
-
static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
PCIBus *bus, PIIX4PMState *s)
{
@@ -565,8 +557,6 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion
*parent,
AcpiCpuHotplug_init(parent, OBJECT(s), &s->gpe_cpu,
PIIX4_CPU_HOTPLUG_IO_BASE);
-s->cpu_added_notifier.notify = piix4_cpu_added_req;
-qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
if (s->acpi_memory_hotplug.is_enabled) {
acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 166edb0..8188630 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -23,8 +23,6 @@ typedef struct AcpiCpuHotplug {
void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
AcpiCpuHotplug *g, CPUState *dev, Error **errp);
-void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu);
-
void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
AcpiCpuHotplug *gpe_cpu, uint16_t base);
#endif
--
1.7.7
[Qemu-devel] [PATCH V3 2/7] acpi:ich9: convert cpu hotplug handle to hotplug_handler API
Convert notifier based hotplug handle to hotplug_handler API.
Signed-off-by: Gu Zheng
---
hw/acpi/ich9.c | 14 +++---
include/hw/acpi/ich9.h |1 -
2 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 7b14bbb..c53d4ab 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -209,15 +209,6 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
acpi_pm1_evt_power_down(&pm->acpi_regs);
}
-static void ich9_cpu_added_req(Notifier *n, void *opaque)
-{
-ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier);
-
-assert(pm != NULL);
-AcpiCpuHotplug_add(&pm->acpi_regs.gpe, &pm->gpe_cpu, CPU(opaque));
-acpi_update_sci(&pm->acpi_regs, pm->irq);
-}
-
void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
qemu_irq sci_irq)
{
@@ -246,8 +237,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
&pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
-pm->cpu_added_notifier.notify = ich9_cpu_added_req;
-qemu_register_cpu_added_notifier(&pm->cpu_added_notifier);
if (pm->acpi_memory_hotplug.is_enabled) {
acpi_memory_hotplug_init(pci_address_space_io(lpc_pci),
OBJECT(lpc_pci),
@@ -304,6 +293,9 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState
*dev, Error **errp)
object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, &pm->acpi_memory_hotplug,
dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+acpi_cpu_plug_cb(&pm->acpi_regs, pm->irq, &pm->gpe_cpu,
+ CPU(dev), errp);
} else {
error_setg(errp, "acpi: device plug request for not supported device"
" type: %s", object_get_typename(OBJECT(dev)));
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index 7e42448..fe975e6 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -47,7 +47,6 @@ typedef struct ICH9LPCPMRegs {
Notifier powerdown_notifier;
AcpiCpuHotplug gpe_cpu;
-Notifier cpu_added_notifier;
MemHotplugState acpi_memory_hotplug;
} ICH9LPCPMRegs;
--
1.7.7
[Qemu-devel] [PATCH V3 1/7] acpi/cpu: add cpu hotplug callback function to match hotplug_handler API
---
v2:
-add errp argument to catch error.
-return error instead of aborting if cpu id is invalid.
-make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add.
---
Signed-off-by: Gu Zheng
---
hw/acpi/cpu_hotplug.c | 17 +
include/hw/acpi/cpu_hotplug.h |3 +++
2 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 2ad83a0..dfd6de5 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -36,6 +36,23 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
},
};
+void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+ AcpiCpuHotplug *g, CPUState *cpu, Error **errp)
+{
+CPUClass *k = CPU_GET_CLASS(cpu);
+int64_t cpu_id;
+
+cpu_id = k->get_arch_id(cpu);
+if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
+error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
+return;
+}
+
+AcpiCpuHotplug_add(&ar->gpe, g, cpu);
+
+acpi_update_sci(ar, irq);
+}
+
void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu)
{
CPUClass *k = CPU_GET_CLASS(cpu);
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 9e5d30c..166edb0 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -20,6 +20,9 @@ typedef struct AcpiCpuHotplug {
uint8_t sts[ACPI_GPE_PROC_LEN];
} AcpiCpuHotplug;
+void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+ AcpiCpuHotplug *g, CPUState *dev, Error **errp);
+
void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu);
void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
--
1.7.7
[Qemu-devel] [PATCH V3 0/7] cpu/acpi: convert cpu hot plug to hotplug_handler API
Previously we use cpu_added_notifiers to register cpu hotplug notifier callback which is not able to pass/handle errors, so we switch it to unified hotplug handler API which allows to pass errors and would allow to cancel device_add in case of error. Thanks very much for Igor's review and suggestion. v3: -deal with start-up cpus in pc_cpu_plug as Igor suggested. v2: -Add 3 new patches(5/7,6/7,7/7), delete original patch 5/5. 1/5-->1/7 2/5-->2/7 3/5-->3/7 4/5-->4/7 Patch 1/7: -add errp argument to catch error. -return error instead of aborting if cpu id is invalid. -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add. Patch 3/7: -remove unused AcpiCpuHotplug_add directly. Patch 5/7: -switch the last user of cpu hotplug notifier to hotplug handler API, and remove the unused cpu hotplug notify. Patch 6/7: -split the function rename (just cleanup) into single patch. Patch 7/7: -introduce help function acpi_set_local_sts to keep the bit setting in one place. Gu Zheng (7): acpi/cpu: add cpu hotplug callback function to match hotplug_handler API acpi:ich9: convert cpu hotplug handle to hotplug_handler API acpi:piix4: convert cpu hotplug handle to hotplug_handler API pc: add cpu hotplug handler to PC_MACHINE pc: Update rtc_cmos in pc_cpu_plug cpu-hotplug: rename function for better readability acpi/cpu-hotplug: introduce help function to keep bit setting in one place hw/acpi/cpu_hotplug.c | 35 hw/acpi/ich9.c| 18 -- hw/acpi/piix4.c | 18 +++--- hw/i386/pc.c | 51 + include/hw/acpi/cpu_hotplug.h |7 +++-- include/hw/acpi/ich9.h|1 - include/sysemu/sysemu.h |3 -- qom/cpu.c | 10 8 files changed, 69 insertions(+), 74 deletions(-) -- 1.7.7
Re: [Qemu-devel] [PATCH v15 0/5] qcow2, raw: add preallocation=full and preallocation=falloc
On Tue, Sep 16, 2014 at 12:19:58PM +0200, Kevin Wolf wrote: > Am 16.09.2014 um 12:10 hat Hu Tao geschrieben: > > ping... > > Sorry, forgot to send the mail when I merged it. This is in master now. Thank you very much! Regards, Hu > > Kevin > > > On Fri, Sep 12, 2014 at 05:22:45PM +0800, Hu Tao wrote: > > > ping? > > > > > > On Wed, Sep 10, 2014 at 05:05:44PM +0800, Hu Tao wrote: > > > > This series adds two preallocation mode to qcow2 and raw: > > > > > > > > Option preallocation=full preallocates disk space for image by writing > > > > zeros to disk, this ensures disk space in any cases. > > > > > > > > Option preallocation=falloc preallocates disk space by calling > > > > posix_fallocate(). This is faster than preallocation=full. > > > > > > > > Note: there is a false positive reported by checkpatch.pl to patch 1. > > > > > > > > changes to v14: > > > > > > > > - add detailed commit message to patch 1 > > > > - change the coding style as Eric and Benoît suggested (patch 3) > > > > - use break as Benoît suggested (patch 4) > > > > > > > > changes to v13: > > > > > > > > - rebase (patch 3 in v13 is already in) > > > > - don't convert file size to sector size in hdev_create(), too (patch > > > > 2) > > > > - reintroduce preallocation=falloc. (patch 3) > > > > - split the implementation of preallocation=full in v13 into > > > > preallocation=falloc and preallocation=full (patch 4) > > > > > > > > changes to v12: > > > > > > > > - remove dependence on minimal_blob_size() (patch 6) > > > > - remove preallocation=falloc. (patch 4) > > > > - preallocation=full tries posix_fallocate() first then writing > > > > zeros (patch 5) > > > > - round up file size for all formats (patch 1) > > > > - avoid converting file size for more formats (patch 2) > > > > > > > > changes to v11: > > > > > > > > - fix test case 049 (patch 4) > > > > - unsigned nl2e -> uint64_t nl2e (patch 6) > > > > - use >> instead of / (patch 6) > > > > > > > > changes to v10: > > > > > > > > - PreallocMode is moved from file qapi-schema.json to > > > > qapi/block-core.json > > > > - introdues preallocation=falloc, no changes to preallocation=metadata > > > > - using minimal_blob_size() to calculate metadata size for qcow2 > > > > - indentation fix in file blockdev.c > > > > > > > > changes to v9: > > > > > > > > - use ROUND_UP to do round up > > > > - split the round up into its own patch and add test case > > > > - new patch rename parse_enum_option to qapi_enum_parse and make it > > > > public > > > > - reuse qapi_enum_parse > > > > > > > > changes to v8: > > > > > > > > - round up image file size to nearest sector size > > > > - dont' blindly lose error info > > > > - target for 2.1 rather than 2.0 > > > > - and, rebase to latest git tree > > > > > > > > changes to v5: > > > > > > > > - add `Since 2.0' to PreallocMode > > > > - apply total_size change to raw-win32.c as well > > > > > > > > changes to v4: > > > > > > > > - fix wrong calculation of qcow2 metadata size in v4 > > > > - remove raw_preallocate2() > > > > - better error out path in raw_create() > > > > - fix coding style > > > > > > > > changes to v3: > > > > > > > > - remove bdrv_preallocate and make preallocation a > > > > bdrv_create_file option > > > > - prealloc_mode -> PreallocMode and add it to QAPI > > > > - fix return value in raw_preallocate2 > > > > > > > > changes to v2: > > > > > > > > - Fix comments to v2 by Fam. > > > > - qcow2: first fallocate disk space, then allocate metadata. This > > > > avoids > > > > the problem in v2 that bdrv_preallocate may clear all information in > > > > metadata. This does not necessarily map all data clusters > > > > sequentially > > > > but does keep information in metadata. Peter, is this acceptable? > > > > > > > > > > > > Hu Tao (5): > > > > block: round up file size to nearest sector > > > > block: don't convert file size to sector size > > > > qapi: introduce PreallocMode and new PreallocModes full and falloc. > > > > raw-posix: Add falloc and full preallocation option > > > > qcow2: Add falloc and full preallocation option > > > > > > > > block/archipelago.c | 3 +- > > > > block/cow.c | 3 +- > > > > block/gluster.c | 9 ++-- > > > > block/iscsi.c| 4 +- > > > > block/nfs.c | 3 +- > > > > block/qcow.c | 7 +-- > > > > block/qcow2.c| 82 +-- > > > > block/qed.c | 3 +- > > > > block/raw-posix.c| 102 > > > > ++- > > > > block/raw-win32.c| 6 +-- > > > > block/rbd.c | 3 +- > > > > block/sheepdog.c | 3 +- > > > > block/ssh.c | 3 +- > > > > block/vdi.c
Re: [Qemu-devel] [PATCH] hmp: fix memory leak at hmp_info_block_jobs()
> From: Markus Armbruster [mailto:arm...@redhat.com] > Sent: Tuesday, September 16, 2014 11:12 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org; kw...@redhat.com; Huangweidong (C); > lcapitul...@redhat.com; stefa...@redhat.com > Subject: Re: [Qemu-devel] [PATCH] hmp: fix memory leak at > hmp_info_block_jobs() > > writes: > > > From: Gonglei > > > > Signed-off-by: Gonglei > > --- > > hmp.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/hmp.c b/hmp.c > > index 40a90da..31fb6a1 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -679,6 +679,8 @@ void hmp_info_block_jobs(Monitor *mon, const > QDict *qdict) > > } > > list = list->next; > > } > > + > > +qapi_free_BlockJobInfoList(list); > > } > > > > void hmp_info_tpm(Monitor *mon, const QDict *qdict) > > Reviewed-by: Markus Armbruster Thanks for review. :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v2 0/2] usb: Don't use qerror_report
> From: Markus Armbruster [mailto:arm...@redhat.com]
> Sent: Wednesday, September 17, 2014 12:16 AM
> To: Gerd Hoffmann
> Cc: Gonglei (Arei); Huangweidong (C); qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH v2 0/2] usb: Don't use qerror_report
>
> Gerd Hoffmann writes:
>
> > Hi,
> >
> >> > Gonglei (2):
> >> > redirect.c: Don't use qerror_report()
> >> > dev-network: Don't use qerror_report_err()
> >
> >> Hi, Gerd
> >> Would you like to apply this patch series at present? Thanks!
> >
> > Picked it up now. Originally dropped due to review comments from paolo,
> > but switching all usb over to realize is a bigger project indeed.
>
> Please don't, it degrades QMP errors. Example:
>
> { "execute": "device_add", "arguments": { "driver": "usb-redir" } }
>
> Reply before:
>
> {"error": {"class": "GenericError", "desc": "Parameter 'chardev' is
> missing"}}
>
> After:
>
> {"error": {"class": "GenericError", "desc": "Device initialization
> failed."}}
>
> with "Parameter 'chardev' is missing" barfed to stderr.
Hum.. Thanks for your information :)
Gerd, please drop this patch series, Thanks!
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH v2 0/2] usb: Don't use qerror_report
> From: Markus Armbruster [mailto:arm...@redhat.com] > Sent: Tuesday, September 16, 2014 10:26 PM > To: Paolo Bonzini > Cc: Gonglei (Arei); qemu-devel@nongnu.org; Huangweidong (C); > kra...@redhat.com > Subject: Re: [Qemu-devel] [PATCH v2 0/2] usb: Don't use qerror_report > > Paolo Bonzini writes: > > > Il 16/09/2014 15:54, Gonglei (Arei) ha scritto: > >>> -Original Message- > >>> From: Gonglei (Arei) > >>> Sent: Friday, September 12, 2014 3:31 PM > >>> To: qemu-devel@nongnu.org > >>> Cc: kra...@redhat.com; Huangweidong (C); arm...@redhat.com; Gonglei > >>> (Arei) > >>> Subject: [PATCH v2 0/2] usb: Don't use qerror_report > >>> > >>> From: Gonglei > >>> > >>> qerror_report() is a transitional interface to help with converting > >>> existing HMP commands to QMP. It should not be used elsewhere. > >>> > >>> v2 -> v1: > >>> - update including head files, remove qerror.h and monitor.h, > >>>add error-report.h (Markus) > >>> - add 'Reviewed-by' tag. > >>> > >>> Gonglei (2): > >>> redirect.c: Don't use qerror_report() > >>> dev-network: Don't use qerror_report_err() > >>> > >>> hw/usb/dev-network.c | 4 ++-- > >>> hw/usb/redirect.c| 8 > >>> 2 files changed, 6 insertions(+), 6 deletions(-) > >>> > >>> -- > >>> 1.7.12.4 > >>> > >> Hi, Gerd > >> Would you like to apply this patch series at present? Thanks! > > > > I'm not sure what the improvement is? > > The improvement is obvious: three calls of qerror_report() gone. > > The regression isn't as obvious (I missed it, but fortunately Paolo > spotted it): when we run in QMP context, we screw up the error reply. > > Therefore, we can't take these patches. We really have to convert to > realize to get rid of qerror_report() here. OK, understand it. Thank you, guys :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH] virtio-balloon: Add some trace events
Hi, Ping... Is this acceptable? Thanks, zhanghailiang On 2014/9/12 15:06, zhanghailiang wrote: Add some trace events for easier debugging Signed-off-by: zhanghailiang --- hw/virtio/virtio-balloon.c | 6 ++ trace-events | 4 2 files changed, 10 insertions(+) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 2c30b3d..112f97c 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -25,6 +25,7 @@ #include "exec/address-spaces.h" #include "qapi/visitor.h" #include "qapi-event.h" +#include "trace.h" #if defined(__linux__) #include @@ -217,6 +218,8 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq) if (!int128_nz(section.size) || !memory_region_is_ram(section.mr)) continue; +trace_virtio_balloon_handle_output(memory_region_name(section.mr), + pa); /* Using memory_region_get_ram_ptr is bending the rules a bit, but should be OK because we only want a single page. */ addr = section.offset_within_region; @@ -280,6 +283,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) config.num_pages = cpu_to_le32(dev->num_pages); config.actual = cpu_to_le32(dev->actual); +trace_virtio_balloon_get_config(config.num_pages, config.actual); memcpy(config_data,&config, sizeof(struct virtio_balloon_config)); } @@ -296,6 +300,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, ((ram_addr_t) dev->actual<< VIRTIO_BALLOON_PFN_SHIFT), &error_abort); } +trace_virtio_balloon_set_config(dev->actual, oldactual); } static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f) @@ -323,6 +328,7 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target) dev->num_pages = (ram_size - target)>> VIRTIO_BALLOON_PFN_SHIFT; virtio_notify_config(vdev); } +trace_virtio_balloon_to_target(target, dev->num_pages); } static void virtio_balloon_save(QEMUFile *f, void *opaque) diff --git a/trace-events b/trace-events index fb58963..fedd7e7 100644 --- a/trace-events +++ b/trace-events @@ -143,6 +143,10 @@ cpu_out(unsigned int addr, unsigned int val) "addr %#x value %u" # balloon.c # Since requests are raised via monitor, not many tracepoints are needed. balloon_event(void *opaque, unsigned long addr) "opaque %p addr %lu" +virtio_balloon_handle_output(const char *name, uint64_t gpa) "setion name: %s gpa: %"PRIx64"" +virtio_balloon_get_config(uint32_t num_pages, uint32_t acutal) "num_pages: %d acutal: %d" +virtio_balloon_set_config(uint32_t acutal, uint32_t oldacutal) "acutal: %d oldacutal: %d" +virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon target: %"PRIx64" num_pages: %d" # hw/intc/apic_common.c cpu_set_apic_base(uint64_t val) "%016"PRIx64
Re: [Qemu-devel] [PATCH v2 0/5] Convert remaining legacy chardevs to QAPI
On 9 September 2014 01:11, Peter Maydell wrote:
> On 9 September 2014 07:47, Paolo Bonzini wrote:
>> Il 02/09/2014 12:24, Peter Maydell ha scritto:
>>> This patchset converts the two remaining legacy chardevs
>>> ('socket' and 'udp') to use the new-style parse/kind
>>> mechanisms, and removes all the no-longer-required
>>> legacy machinery.
>
>>> Peter Maydell (5):
>>> qemu-char: Convert socket backend to QAPI
>>> util/qemu-sockets.c: Support specifying IPv4 or IPv6 in socket_dgram()
>>> qemu-char: Convert udp backend to QAPI
>>> qemu-char: Remove register_char_driver() machinery
>>> qemu-char: Rename register_char_driver_qapi() to
>>> register_char_driver()
>> Hi Peter, are you going to apply these directly?
>
> I guess I could; I was hoping for some review first though :-)
Well, no more review, so I've applied these to master.
-- PMM
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
On 09/16/2014 11:41 AM, Richard Henderson wrote: >> In practice, only 3 to 7 are---hence my original attempt at using some >> kind of FIFO caching: >> >>user mode, translation enabled >>kernel mode, paging disabled >>kernel mode, paging enabled >>supervisor mode, paging disabled >>supervisor mode, paging enabled >> >> Plus perhaps kernel and supervisor mode with only data paging enabled. >> >> You could lump together the IR!=0, DR!=0 cases, and flush that one TLB >> index if the IR/DR pair changes with respect to the last time. This >> would use 6 indices. > > I think I would prefer a solution that uses 6 indicies, as will not cause env > to overflow 64k, and not require that any tcg backends be updated. ... alternately ... What if instead of having a "mmu_index" for the mmu arrays, we have a pointer to the "mmu context". This does imply an extra memory load on the fast path, but probably not an extra instruction. With this, we can suddenly afford to have a relatively large number of mmu contexts, with which we could implement address space numbers for relevant targets. It is, of course, a much larger change, but perhaps it's of larger benefit. r~
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
On 09/16/2014 11:49 AM, Peter Maydell wrote:
> On 16 September 2014 10:20, Tom Musta wrote:
>>
>> 1389 /* Compensate for very large offsets. */
>> 1390 if (add_off >= 0x8000) {
>> 1391 /* Most target env are smaller than 32k; none are larger
>> than 64k.
>> 1392 Simplify the logic here merely to offset by 0x7ff0,
>> giving us a
>> 1393 range just shy of 64k. Check this assumption. */
>> 1394 QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
>> 1395 tlb_table[NB_MMU_MODES - 1][1])
>> 1396> 0x7ff0 + 0x7fff);
>> 1397 tcg_out32(s, ADDI | TAI(TCG_REG_TMP1, base, 0x7ff0));
>> 1398 base = TCG_REG_TMP1;
>> 1399 cmp_off -= 0x7ff0;
>> 1400 add_off -= 0x7ff0;
>> 1401 }
>
> Is it possible to promote this BUILD_BUG_ON from "only on
> PPC hosts" to "on all builds" ? It's really checking a
> property of the target CPU's code, not a property of
> the TCG backend, and I bet a lot of our backends don't
> get built very often so we could easily miss breakage.
> I guess you'd need to define and check a worst-case value
> in a common header somewhere.
Meh. It is a property of the tcg backend, in that it is a property of the code
that immediately follows. And that's what makes the BUG_ON clear and obvious,
IMO.
For what it's worth, ppc as written has the smallest constraint of the current
backends, and I'm fairly confident that'll get built often-ish.
If you've got a rearrangement that puts the assert somewhere else, and keeps
the magic numbers understandable... I'll certainly have a look, but I don't see
how to retain the obviousness with a different placement.
r~
Re: [Qemu-devel] [PATCH v6 00/16] KVM platform device passthrough
On Tue, 2014-09-16 at 14:51 -0600, Alex Williamson wrote: > On Tue, 2014-09-16 at 00:01 +0200, Eric Auger wrote: > > On 09/12/2014 01:05 AM, Christoffer Dall wrote: > > > On Thu, Sep 11, 2014 at 04:51:14PM -0600, Alex Williamson wrote: > > >> On Thu, 2014-09-11 at 15:23 -0700, Christoffer Dall wrote: > > >>> On Thu, Sep 11, 2014 at 04:14:09PM -0600, Alex Williamson wrote: > > On Tue, 2014-09-09 at 08:31 +0100, Eric Auger wrote: > > > This RFC series aims at enabling KVM platform device passthrough. > > > It implements a VFIO platform device, derived from VFIO PCI device. > > > > > > The VFIO platform device uses the host VFIO platform driver which must > > > be bound to the assigned device prior to the QEMU system start. > > > > > > - the guest can directly access the device register space > > > - assigned device IRQs are transparently routed to the guest by > > > QEMU/KVM (3 methods currently are supported: user-level eventfd > > > handling, irqfd, forwarded IRQs) > > > - iommu is transparently programmed to prevent the device from > > > accessing physical pages outside of the guest address space > > > > > > This patch series is made of the following patch files: > > > > > > 1-7) Modifications to PCI code to prepare for VFIO platform device > > > 8) split of PCI specific code and generic code (move) > > > 9-11) creation of the VFIO calxeda xgmac platform device, without > > > irqfd > > > support (MMIO direct access and IRQ assignment). > > > 12) fake injection test modality (to test multiple IRQ) > > > 13) addition of irqfd/virqfd support > > > 14-16) forwarded IRQ > > > > > > Dependency List: > > > > > > QEMU dependencies: > > > [1] [PATCH v2 0/9] Dynamic sysbus device allocation support, Alex Graf > > > http://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00047.html > > > [2] [RFC v3] machvirt dynamic sysbus device instantiation, Eric Auger > > > [3] [PATCH v2 0/2] actual checks of KVM_CAP_IRQFD and > > > KVM_CAP_IRQFD_RESAMPLE, > > > Eric Auger > > > > > > http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00589.html > > > [4] [RFC] vfio: migration to trace points, Eric Auger > > > > > > http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00569.html > > > > > > Kernel Dependencies: > > > [5] [RFC Patch v6 0/20] VFIO support for platform devices, Antonios > > > Motakis > > > https://www.mail-archive.com/kvm@vger.kernel.org/msg103247.html > > > [6] [PATCH v3] ARM: KVM: add irqfd support, Eric Auger > > > https://lkml.org/lkml/2014/9/1/141 > > > [7] arm/arm64: KVM: Various VGIC cleanups and improvements, > > > Christoffer Dall > > > http://comments.gmane.org/gmane.linux.ports.arm.kernel/340430 > > > [8] [RFC v2 0/9] KVM-VFIO IRQ forward control, Eric Auger > > > https://lkml.org/lkml/2014/9/1/344 > > > [9] [RFC PATCH 0/9] ARM: Forwarding physical interrupts to a guest VM, > > > Marc Zyngier > > > http://lwn.net/Articles/603514/ > > > > > > kernel pieces can be found at: > > > http://git.linaro.org/people/eric.auger/linux.git > > > (branch 3.17rc3_irqfd_forward_integ_v2) > > > QEMU pieces can be found at: > > > http://git.linaro.org/people/eric.auger/qemu.git (branch > > > vfio_integ_v6) > > > > > > The patch series was tested on Calxeda Midway (ARMv7) where one xgmac > > > is assigned to KVM host while the second one is assigned to the guest. > > > Reworked PCI device is not tested. > > > > > > Wiki for Calxeda Midway setup: > > > https://wiki.linaro.org/LEG/Engineering/Virtualization/Platform_Device_Passthrough_on_Midway > > > > > > History: > > > > > > v5->v6: > > > - rebase on 2.1rc5 PCI code > > > - forwarded IRQ first integraton > > > > Why? Are there acceleration paths that you're concerned cannot be > > implemented or we do not already have a proof of concept for? The base > > kernel patch series you depend on is 3 months old yet this series > > continues to grow and add new dependencies. Please let's prioritize > > getting something upstream instead of adding more blockers to prevent > > that. Thanks, > > > > >>> I'm not exactly sure what this changelog line was referring to > > >>> (depending on Marc's forwarding IRQ patches?), but just want to add that > > >>> there are a number of dependencies for the GIC that need to go in as > > >>> well (should happen within a few weeks), but I think it's unlikely that > > >>> the IRQ forwarding stuff goes in for v3.18 at this point. > > >>> > > >>> It may make sense as you suggest to keep that part out of this patch set > > >>> and something merged sooner as opposed to later, but I'm too jet-lagged > > >>> to completely understand if that's going to
Re: [Qemu-devel] [PATCH v6 00/16] KVM platform device passthrough
On 09/16/2014 10:51 PM, Alex Williamson wrote: > On Tue, 2014-09-16 at 00:01 +0200, Eric Auger wrote: >> On 09/12/2014 01:05 AM, Christoffer Dall wrote: >>> On Thu, Sep 11, 2014 at 04:51:14PM -0600, Alex Williamson wrote: On Thu, 2014-09-11 at 15:23 -0700, Christoffer Dall wrote: > On Thu, Sep 11, 2014 at 04:14:09PM -0600, Alex Williamson wrote: >> On Tue, 2014-09-09 at 08:31 +0100, Eric Auger wrote: >>> This RFC series aims at enabling KVM platform device passthrough. >>> It implements a VFIO platform device, derived from VFIO PCI device. >>> >>> The VFIO platform device uses the host VFIO platform driver which must >>> be bound to the assigned device prior to the QEMU system start. >>> >>> - the guest can directly access the device register space >>> - assigned device IRQs are transparently routed to the guest by >>> QEMU/KVM (3 methods currently are supported: user-level eventfd >>> handling, irqfd, forwarded IRQs) >>> - iommu is transparently programmed to prevent the device from >>> accessing physical pages outside of the guest address space >>> >>> This patch series is made of the following patch files: >>> >>> 1-7) Modifications to PCI code to prepare for VFIO platform device >>> 8) split of PCI specific code and generic code (move) >>> 9-11) creation of the VFIO calxeda xgmac platform device, without irqfd >>> support (MMIO direct access and IRQ assignment). >>> 12) fake injection test modality (to test multiple IRQ) >>> 13) addition of irqfd/virqfd support >>> 14-16) forwarded IRQ >>> >>> Dependency List: >>> >>> QEMU dependencies: >>> [1] [PATCH v2 0/9] Dynamic sysbus device allocation support, Alex Graf >>> http://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00047.html >>> [2] [RFC v3] machvirt dynamic sysbus device instantiation, Eric Auger >>> [3] [PATCH v2 0/2] actual checks of KVM_CAP_IRQFD and >>> KVM_CAP_IRQFD_RESAMPLE, >>> Eric Auger >>> >>> http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00589.html >>> [4] [RFC] vfio: migration to trace points, Eric Auger >>> >>> http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00569.html >>> >>> Kernel Dependencies: >>> [5] [RFC Patch v6 0/20] VFIO support for platform devices, Antonios >>> Motakis >>> https://www.mail-archive.com/kvm@vger.kernel.org/msg103247.html >>> [6] [PATCH v3] ARM: KVM: add irqfd support, Eric Auger >>> https://lkml.org/lkml/2014/9/1/141 >>> [7] arm/arm64: KVM: Various VGIC cleanups and improvements, Christoffer >>> Dall >>> http://comments.gmane.org/gmane.linux.ports.arm.kernel/340430 >>> [8] [RFC v2 0/9] KVM-VFIO IRQ forward control, Eric Auger >>> https://lkml.org/lkml/2014/9/1/344 >>> [9] [RFC PATCH 0/9] ARM: Forwarding physical interrupts to a guest VM, >>> Marc Zyngier >>> http://lwn.net/Articles/603514/ >>> >>> kernel pieces can be found at: >>> http://git.linaro.org/people/eric.auger/linux.git >>> (branch 3.17rc3_irqfd_forward_integ_v2) >>> QEMU pieces can be found at: >>> http://git.linaro.org/people/eric.auger/qemu.git (branch vfio_integ_v6) >>> >>> The patch series was tested on Calxeda Midway (ARMv7) where one xgmac >>> is assigned to KVM host while the second one is assigned to the guest. >>> Reworked PCI device is not tested. >>> >>> Wiki for Calxeda Midway setup: >>> https://wiki.linaro.org/LEG/Engineering/Virtualization/Platform_Device_Passthrough_on_Midway >>> >>> History: >>> >>> v5->v6: >>> - rebase on 2.1rc5 PCI code >>> - forwarded IRQ first integraton >> >> Why? Are there acceleration paths that you're concerned cannot be >> implemented or we do not already have a proof of concept for? The base >> kernel patch series you depend on is 3 months old yet this series >> continues to grow and add new dependencies. Please let's prioritize >> getting something upstream instead of adding more blockers to prevent >> that. Thanks, >> > I'm not exactly sure what this changelog line was referring to > (depending on Marc's forwarding IRQ patches?), but just want to add that > there are a number of dependencies for the GIC that need to go in as > well (should happen within a few weeks), but I think it's unlikely that > the IRQ forwarding stuff goes in for v3.18 at this point. > > It may make sense as you suggest to keep that part out of this patch set > and something merged sooner as opposed to later, but I'm too jet-lagged > to completely understand if that's going to be a horrible mess. The point is that we're on v6 of a patch series and its first non-RFC posting and we're rolling in a first pass at a QEMU implementation that depends on a contested kernel RFC,
Re: [Qemu-devel] [PATCH v6 00/16] KVM platform device passthrough
On Tue, 2014-09-16 at 00:01 +0200, Eric Auger wrote: > On 09/12/2014 01:05 AM, Christoffer Dall wrote: > > On Thu, Sep 11, 2014 at 04:51:14PM -0600, Alex Williamson wrote: > >> On Thu, 2014-09-11 at 15:23 -0700, Christoffer Dall wrote: > >>> On Thu, Sep 11, 2014 at 04:14:09PM -0600, Alex Williamson wrote: > On Tue, 2014-09-09 at 08:31 +0100, Eric Auger wrote: > > This RFC series aims at enabling KVM platform device passthrough. > > It implements a VFIO platform device, derived from VFIO PCI device. > > > > The VFIO platform device uses the host VFIO platform driver which must > > be bound to the assigned device prior to the QEMU system start. > > > > - the guest can directly access the device register space > > - assigned device IRQs are transparently routed to the guest by > > QEMU/KVM (3 methods currently are supported: user-level eventfd > > handling, irqfd, forwarded IRQs) > > - iommu is transparently programmed to prevent the device from > > accessing physical pages outside of the guest address space > > > > This patch series is made of the following patch files: > > > > 1-7) Modifications to PCI code to prepare for VFIO platform device > > 8) split of PCI specific code and generic code (move) > > 9-11) creation of the VFIO calxeda xgmac platform device, without irqfd > > support (MMIO direct access and IRQ assignment). > > 12) fake injection test modality (to test multiple IRQ) > > 13) addition of irqfd/virqfd support > > 14-16) forwarded IRQ > > > > Dependency List: > > > > QEMU dependencies: > > [1] [PATCH v2 0/9] Dynamic sysbus device allocation support, Alex Graf > > http://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00047.html > > [2] [RFC v3] machvirt dynamic sysbus device instantiation, Eric Auger > > [3] [PATCH v2 0/2] actual checks of KVM_CAP_IRQFD and > > KVM_CAP_IRQFD_RESAMPLE, > > Eric Auger > > > > http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00589.html > > [4] [RFC] vfio: migration to trace points, Eric Auger > > > > http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00569.html > > > > Kernel Dependencies: > > [5] [RFC Patch v6 0/20] VFIO support for platform devices, Antonios > > Motakis > > https://www.mail-archive.com/kvm@vger.kernel.org/msg103247.html > > [6] [PATCH v3] ARM: KVM: add irqfd support, Eric Auger > > https://lkml.org/lkml/2014/9/1/141 > > [7] arm/arm64: KVM: Various VGIC cleanups and improvements, Christoffer > > Dall > > http://comments.gmane.org/gmane.linux.ports.arm.kernel/340430 > > [8] [RFC v2 0/9] KVM-VFIO IRQ forward control, Eric Auger > > https://lkml.org/lkml/2014/9/1/344 > > [9] [RFC PATCH 0/9] ARM: Forwarding physical interrupts to a guest VM, > > Marc Zyngier > > http://lwn.net/Articles/603514/ > > > > kernel pieces can be found at: > > http://git.linaro.org/people/eric.auger/linux.git > > (branch 3.17rc3_irqfd_forward_integ_v2) > > QEMU pieces can be found at: > > http://git.linaro.org/people/eric.auger/qemu.git (branch vfio_integ_v6) > > > > The patch series was tested on Calxeda Midway (ARMv7) where one xgmac > > is assigned to KVM host while the second one is assigned to the guest. > > Reworked PCI device is not tested. > > > > Wiki for Calxeda Midway setup: > > https://wiki.linaro.org/LEG/Engineering/Virtualization/Platform_Device_Passthrough_on_Midway > > > > History: > > > > v5->v6: > > - rebase on 2.1rc5 PCI code > > - forwarded IRQ first integraton > > Why? Are there acceleration paths that you're concerned cannot be > implemented or we do not already have a proof of concept for? The base > kernel patch series you depend on is 3 months old yet this series > continues to grow and add new dependencies. Please let's prioritize > getting something upstream instead of adding more blockers to prevent > that. Thanks, > > >>> I'm not exactly sure what this changelog line was referring to > >>> (depending on Marc's forwarding IRQ patches?), but just want to add that > >>> there are a number of dependencies for the GIC that need to go in as > >>> well (should happen within a few weeks), but I think it's unlikely that > >>> the IRQ forwarding stuff goes in for v3.18 at this point. > >>> > >>> It may make sense as you suggest to keep that part out of this patch set > >>> and something merged sooner as opposed to later, but I'm too jet-lagged > >>> to completely understand if that's going to be a horrible mess. > >> > >> The point is that we're on v6 of a patch series and its first non-RFC > >> posting and we're rolling in a first pass at a QEMU implementation that > >> depends on a contested kernel RFC, which depends on another stagnant > >> kernel RFC.
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
On 09/16/2014 12:31 PM, Paolo Bonzini wrote: >> Change legacy_name to point to a detailed human-readable >> description of the type? >> E.g. "Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF"? > > If libvirt can cope with > > e1000.mac=str (Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF) Isn't this output only available under -help? Libvirt only cares about QMP listings of devices and their properties, so improving the human interface should have no negative impact to the machine interface. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3] async: aio_context_new(): Handle event_notifier_init failure
The Tuesday 16 Sep 2014 à 13:40:24 (-0600), Eric Blake wrote :
> On 09/16/2014 12:04 PM, Chrysostomos Nanakos wrote:
> > If event_notifier_init fails QEMU exits without printing
> > any error information to the user. This commit adds an error
> > message on failure:
> >
> > # qemu [...]
>
> Showing the actual command line you used would be helpful.
>
> > qemu: Failed to initialize event notifier: Too many open files in system
> >
> > Signed-off-by: Chrysostomos Nanakos
> > ---
> > async.c | 16 +++-
> > include/block/aio.h |2 +-
> > include/qemu/main-loop.h |2 +-
> > iothread.c | 11 ++-
> > main-loop.c |9 +++--
> > qemu-img.c |8 +++-
> > qemu-io.c|7 ++-
> > qemu-nbd.c |6 +-
> > tests/test-aio.c | 10 +-
> > tests/test-thread-pool.c | 10 +-
> > tests/test-throttle.c| 10 +-
> > vl.c |5 +++--
> > 12 files changed, 78 insertions(+), 18 deletions(-)
> >
>
> > -AioContext *aio_context_new(void)
> > +AioContext *aio_context_new(Error **errp)
> > {
> > +int ret;
> > AioContext *ctx;
> > ctx = (AioContext *) g_source_new(&aio_source_funcs,
> > sizeof(AioContext));
> > +ret = event_notifier_init(&ctx->notifier, false);
> > +if (ret < 0) {
> > +g_source_destroy(&ctx->source);
>
> Does g_source_destroy() guarantee that errno is unmolested? If not,
>
> > +error_setg_errno(errp, -ret, "Failed to initialize event
> > notifier");
Actually -ret is passed to error_setg_errno.
See.
void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
const char *fmt, ...)
and
if (os_errno != 0) {
err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno));
> Use of \' inside "" is unusual. (Multiple times in your patch)
My fault.
Re: [Qemu-devel] [PATCH v3] async: aio_context_new(): Handle event_notifier_init failure
On 09/16/2014 12:04 PM, Chrysostomos Nanakos wrote:
> If event_notifier_init fails QEMU exits without printing
> any error information to the user. This commit adds an error
> message on failure:
>
> # qemu [...]
Showing the actual command line you used would be helpful.
> qemu: Failed to initialize event notifier: Too many open files in system
>
> Signed-off-by: Chrysostomos Nanakos
> ---
> async.c | 16 +++-
> include/block/aio.h |2 +-
> include/qemu/main-loop.h |2 +-
> iothread.c | 11 ++-
> main-loop.c |9 +++--
> qemu-img.c |8 +++-
> qemu-io.c|7 ++-
> qemu-nbd.c |6 +-
> tests/test-aio.c | 10 +-
> tests/test-thread-pool.c | 10 +-
> tests/test-throttle.c| 10 +-
> vl.c |5 +++--
> 12 files changed, 78 insertions(+), 18 deletions(-)
>
> -AioContext *aio_context_new(void)
> +AioContext *aio_context_new(Error **errp)
> {
> +int ret;
> AioContext *ctx;
> ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
> +ret = event_notifier_init(&ctx->notifier, false);
> +if (ret < 0) {
> +g_source_destroy(&ctx->source);
Does g_source_destroy() guarantee that errno is unmolested? If not,
> +error_setg_errno(errp, -ret, "Failed to initialize event notifier");
then this logs the wrong error. Swap the lines to be safe.
> +return NULL;
> +}
> +aio_set_event_notifier(ctx, &ctx->notifier,
> + (EventNotifierHandler *)
> + event_notifier_test_and_clear);
> ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
> ctx->thread_pool = NULL;
> qemu_mutex_init(&ctx->bh_lock);
> rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx);
> -event_notifier_init(&ctx->notifier, false);
Is hoisting the event notifier init to occur before the mutex init going
to cause any grief?
> +++ b/main-loop.c
> @@ -138,8 +139,12 @@ int qemu_init_main_loop(void)
> return ret;
> }
>
> +qemu_aio_context = aio_context_new(&local_error);
> +if (!qemu_aio_context) {
> +error_propagate(errp, local_error);
> +return -1;
> +}
Can the earlier call to qemu_signal_init() consume any resources that
are now leaked?
> @@ -205,10 +206,17 @@ static void test_cancel(void)
> int main(int argc, char **argv)
> {
> int ret;
> +Error *local_error;
>
> init_clocks();
>
> -ctx = aio_context_new();
> +ctx = aio_context_new(&local_error);
> +if (!ctx) {
> +error_report("Failed to create AIO Context: \'%s\'",
Use of \' inside "" is unusual. (Multiple times in your patch)
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH] block: vhdx - fix reading beyond pointer during image creation
In vhdx_create_metadata(), we allocate 40 bytes to entry_buffer for
the various metadata table entries. However, we write out 64kB from
that buffer into the new file. Only write out the correct 40 bytes.
Signed-off-by: Jeff Cody
---
block/vhdx.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index 796b7bd..b52ec32 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1407,6 +1407,12 @@ exit:
return ret;
}
+#define VHDX_METADATA_ENTRY_BUFFER_SIZE \
+(sizeof(VHDXFileParameters)
+\
+ sizeof(VHDXVirtualDiskSize)
+\
+ sizeof(VHDXPage83Data)
+\
+ sizeof(VHDXVirtualDiskLogicalSectorSize)
+\
+ sizeof(VHDXVirtualDiskPhysicalSectorSize))
/*
* Create the Metadata entries.
@@ -1445,11 +1451,7 @@ static int vhdx_create_new_metadata(BlockDriverState *bs,
VHDXVirtualDiskLogicalSectorSize *mt_log_sector_size;
VHDXVirtualDiskPhysicalSectorSize *mt_phys_sector_size;
-entry_buffer = g_malloc0(sizeof(VHDXFileParameters) +
- sizeof(VHDXVirtualDiskSize) +
- sizeof(VHDXPage83Data) +
- sizeof(VHDXVirtualDiskLogicalSectorSize) +
- sizeof(VHDXVirtualDiskPhysicalSectorSize));
+entry_buffer = g_malloc0(VHDX_METADATA_ENTRY_BUFFER_SIZE);
mt_file_params = entry_buffer;
offset += sizeof(VHDXFileParameters);
@@ -1530,7 +1532,7 @@ static int vhdx_create_new_metadata(BlockDriverState *bs,
}
ret = bdrv_pwrite(bs, metadata_offset + (64 * KiB), entry_buffer,
- VHDX_HEADER_BLOCK_SIZE);
+ VHDX_METADATA_ENTRY_BUFFER_SIZE);
if (ret < 0) {
goto exit;
}
@@ -1725,7 +1727,6 @@ static int vhdx_create_new_region_table(BlockDriverState
*bs,
goto exit;
}
-
exit:
g_free(s);
g_free(buffer);
@@ -1876,7 +1877,6 @@ static int vhdx_create(const char *filename, QemuOpts
*opts, Error **errp)
}
-
delete_and_exit:
bdrv_unref(bs);
exit:
--
1.9.3
Re: [Qemu-devel] [PATCH 07/23] target-xtensa: Use cpu_exec_interrupt qom hook
On 09/16/2014 11:18 AM, Alex Bennée wrote: > Do this mean when all this unwinding has occured cc->do_interrupt will > no longer be needed at all entries will go in via > $ARCH_cpu_exec_interrupt? No. Don't forget the call to do_interrupt in the block immediately following the sigsetjmp. And even ignoring that, some targets (like arm) have several different settings for do_interrupt, while they share a cpu_exec_interrupt hook. r~
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
On Tue, Sep 16, 2014 at 10:01:19PM +0300, Michael S. Tsirkin wrote: > On Tue, Sep 16, 2014 at 08:31:08PM +0200, Paolo Bonzini wrote: > > Il 16/09/2014 18:56, Michael S. Tsirkin ha scritto: > > > On Tue, Sep 16, 2014 at 06:27:51PM +0200, Paolo Bonzini wrote: > > >> Il 16/09/2014 18:26, Michael S. Tsirkin ha scritto: > > >>> Right so types should be explicit. > > >>> If an arbitrary string isn't allowed, this should be documented. > > >>> It's not great as is: what's the format for macaddr? AA:BB:CC:DD:EE:FF? > > >>> aa:bb:cc:dd:ee:ff? aabbccddeeff? 0xaabbccddeeff? > > >>> But just saying "string" is going in the wrong direction imho. > > >> > > >> That's the purpose of documentation (docs/qdev-device-use.txt), > > > > > > That's not user documentation, that's developer documentation, > > > isn't it? > > > > It's user documentation. It's not distributed because we suck at > > documentation. > > > > >> and even > > >> then is better done with examples. I don't think doing it in -device > > >> foo,help (which I'm not even sure is particularly helpful. > > > > > > -device foo,help isn't helpful at all because it does not > > > tell people what does each option do. > > > But it really should be fixed. > > > > Exactly. > > > > >> I'm sympathetic towards fixing the drive->str change, but I have no idea > > >> how to do it. > > > > > > Change legacy_name to point to a detailed human-readable > > > description of the type? > > > E.g. "Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF"? > > > > If libvirt can cope with > > > > e1000.mac=str (Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF) > > > > that would work for me. > > Shouldn't "str" be "string" in HMP? > > Eblake - type is ignored right? Does this mean anything to the right of > = is ignored? > > > > We really really should add description to all properties, too. > > > > This is a huge job. We have hundreds of properties. > > > > Paolo > > Right. If we don't start we won't get there though, will we? > And, we'll keep adding undocumented ones.
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
On Tue, Sep 16, 2014 at 08:31:08PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 18:56, Michael S. Tsirkin ha scritto: > > On Tue, Sep 16, 2014 at 06:27:51PM +0200, Paolo Bonzini wrote: > >> Il 16/09/2014 18:26, Michael S. Tsirkin ha scritto: > >>> Right so types should be explicit. > >>> If an arbitrary string isn't allowed, this should be documented. > >>> It's not great as is: what's the format for macaddr? AA:BB:CC:DD:EE:FF? > >>> aa:bb:cc:dd:ee:ff? aabbccddeeff? 0xaabbccddeeff? > >>> But just saying "string" is going in the wrong direction imho. > >> > >> That's the purpose of documentation (docs/qdev-device-use.txt), > > > > That's not user documentation, that's developer documentation, > > isn't it? > > It's user documentation. It's not distributed because we suck at > documentation. > > >> and even > >> then is better done with examples. I don't think doing it in -device > >> foo,help (which I'm not even sure is particularly helpful. > > > > -device foo,help isn't helpful at all because it does not > > tell people what does each option do. > > But it really should be fixed. > > Exactly. > > >> I'm sympathetic towards fixing the drive->str change, but I have no idea > >> how to do it. > > > > Change legacy_name to point to a detailed human-readable > > description of the type? > > E.g. "Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF"? > > If libvirt can cope with > > e1000.mac=str (Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF) > > that would work for me. Shouldn't "str" be "string" in HMP? Eblake - type is ignored right? Does this mean anything to the right of = is ignored? > > We really really should add description to all properties, too. > > This is a huge job. We have hundreds of properties. > > Paolo Right. If we don't start we won't get there though, will we?
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
On 16 September 2014 10:20, Tom Musta wrote:
>
> 1389 /* Compensate for very large offsets. */
> 1390 if (add_off >= 0x8000) {
> 1391 /* Most target env are smaller than 32k; none are larger than
> 64k.
> 1392 Simplify the logic here merely to offset by 0x7ff0, giving
> us a
> 1393 range just shy of 64k. Check this assumption. */
> 1394 QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
> 1395 tlb_table[NB_MMU_MODES - 1][1])
> 1396> 0x7ff0 + 0x7fff);
> 1397 tcg_out32(s, ADDI | TAI(TCG_REG_TMP1, base, 0x7ff0));
> 1398 base = TCG_REG_TMP1;
> 1399 cmp_off -= 0x7ff0;
> 1400 add_off -= 0x7ff0;
> 1401 }
Is it possible to promote this BUILD_BUG_ON from "only on
PPC hosts" to "on all builds" ? It's really checking a
property of the target CPU's code, not a property of
the TCG backend, and I bet a lot of our backends don't
get built very often so we could easily miss breakage.
I guess you'd need to define and check a worst-case value
in a common header somewhere.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
On 09/16/2014 11:27 AM, Paolo Bonzini wrote: > Il 16/09/2014 20:02, Richard Henderson ha scritto: >> While we could probably fix this for ppc (using addis), it's not nearly so >> easily fixable for arm -- without impacting performance anyway. >> >> Does 96k worth of TLBs really help that much? Are all 12 of them actually >> used? Can we use a more complex encoding scheme for the mmu_idx and use >> less? > > In practice, only 3 to 7 are---hence my original attempt at using some > kind of FIFO caching: > >user mode, translation enabled >kernel mode, paging disabled >kernel mode, paging enabled >supervisor mode, paging disabled >supervisor mode, paging enabled > > Plus perhaps kernel and supervisor mode with only data paging enabled. > > You could lump together the IR!=0, DR!=0 cases, and flush that one TLB > index if the IR/DR pair changes with respect to the last time. This > would use 6 indices. I think I would prefer a solution that uses 6 indicies, as will not cause env to overflow 64k, and not require that any tcg backends be updated. r~
Re: [Qemu-devel] [PATCH 09/23] target-m68k: Use cpu_exec_interrupt qom hook
Richard Henderson writes:
> Since do_interrupt_m68k_hardirq is no longer used outside
> op_helper.c, make it static.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
> ---
> cpu-exec.c | 13 -
> target-m68k/cpu-qom.h | 1 +
> target-m68k/cpu.c | 1 +
> target-m68k/cpu.h | 1 -
> target-m68k/op_helper.c | 22 --
> 5 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 08be521..8ff85ba 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -650,19 +650,6 @@ int cpu_exec(CPUArchState *env)
> next_tb = 0;
> }
> }
> -#elif defined(TARGET_M68K)
> -if (interrupt_request & CPU_INTERRUPT_HARD
> -&& ((env->sr & SR_I) >> SR_I_SHIFT)
> -< env->pending_level) {
> -/* Real hardware gets the interrupt vector via an
> - IACK cycle at this point. Current emulated
> - hardware doesn't rely on this, so we
> - provide/save the vector when the interrupt is
> - first signalled. */
> -cpu->exception_index = env->pending_vector;
> -do_interrupt_m68k_hardirq(env);
> -next_tb = 0;
> -}
> #endif
> /* The target hook has 3 exit conditions:
> False when the interrupt isn't processed,
> diff --git a/target-m68k/cpu-qom.h b/target-m68k/cpu-qom.h
> index 41b14ae..c28e55d 100644
> --- a/target-m68k/cpu-qom.h
> +++ b/target-m68k/cpu-qom.h
> @@ -71,6 +71,7 @@ static inline M68kCPU *m68k_env_get_cpu(CPUM68KState *env)
> #define ENV_OFFSET offsetof(M68kCPU, env)
>
> void m68k_cpu_do_interrupt(CPUState *cpu);
> +bool m68k_cpu_exec_interrupt(CPUState *cpu, int int_req);
> void m68k_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function
> cpu_fprintf,
> int flags);
> hwaddr m68k_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
> index 5a58d51..4cfb725 100644
> --- a/target-m68k/cpu.c
> +++ b/target-m68k/cpu.c
> @@ -196,6 +196,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void
> *data)
> cc->class_by_name = m68k_cpu_class_by_name;
> cc->has_work = m68k_cpu_has_work;
> cc->do_interrupt = m68k_cpu_do_interrupt;
> +cc->cpu_exec_interrupt = m68k_cpu_exec_interrupt;
> cc->dump_state = m68k_cpu_dump_state;
> cc->set_pc = m68k_cpu_set_pc;
> cc->gdb_read_register = m68k_cpu_gdb_read_register;
> diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
> index 6e4001d..f67bbcc 100644
> --- a/target-m68k/cpu.h
> +++ b/target-m68k/cpu.h
> @@ -120,7 +120,6 @@ void m68k_tcg_init(void);
> void m68k_cpu_init_gdb(M68kCPU *cpu);
> M68kCPU *cpu_m68k_init(const char *cpu_model);
> int cpu_m68k_exec(CPUM68KState *s);
> -void do_interrupt_m68k_hardirq(CPUM68KState *env1);
> /* you can call this signal handler from your SIGBUS and SIGSEGV
> signal handlers to inform the virtual CPU of exceptions. non zero
> is returned if the signal was handled by the virtual CPU. */
> diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
> index 9dd3e74..06661f5 100644
> --- a/target-m68k/op_helper.c
> +++ b/target-m68k/op_helper.c
> @@ -27,7 +27,7 @@ void m68k_cpu_do_interrupt(CPUState *cs)
> cs->exception_index = -1;
> }
>
> -void do_interrupt_m68k_hardirq(CPUM68KState *env)
> +static inline void do_interrupt_m68k_hardirq(CPUM68KState *env)
> {
> }
>
> @@ -141,12 +141,30 @@ void m68k_cpu_do_interrupt(CPUState *cs)
> do_interrupt_all(env, 0);
> }
>
> -void do_interrupt_m68k_hardirq(CPUM68KState *env)
> +static inline void do_interrupt_m68k_hardirq(CPUM68KState *env)
> {
> do_interrupt_all(env, 1);
> }
> #endif
>
> +bool m68k_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> +{
> +M68kCPU *cpu = M68K_CPU(cs);
> +CPUM68KState *env = &cpu->env;
> +
> +if (interrupt_request & CPU_INTERRUPT_HARD
> +&& ((env->sr & SR_I) >> SR_I_SHIFT) < env->pending_level) {
> +/* Real hardware gets the interrupt vector via an IACK cycle
> + at this point. Current emulated hardware doesn't rely on
> + this, so we provide/save the vector when the interrupt is
> + first signalled. */
> +cs->exception_index = env->pending_vector;
> +do_interrupt_m68k_hardirq(env);
> +return true;
> +}
> +return false;
> +}
> +
> static void raise_exception(CPUM68KState *env, int tt)
> {
> CPUState *cs = CPU(m68k_env_get_cpu(env));
--
Alex Bennée
Re: [Qemu-devel] [PATCH 08/23] target-s390x: Use cpu_exec_interrupt qom hook
Richard Henderson writes:
> Cc: Alexander Graf
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
> ---
> cpu-exec.c | 6 --
> target-s390x/cpu-qom.h | 1 +
> target-s390x/cpu.c | 1 +
> target-s390x/helper.c | 13 +
> 4 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 304867d..08be521 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -663,12 +663,6 @@ int cpu_exec(CPUArchState *env)
> do_interrupt_m68k_hardirq(env);
> next_tb = 0;
> }
> -#elif defined(TARGET_S390X) && !defined(CONFIG_USER_ONLY)
> -if ((interrupt_request & CPU_INTERRUPT_HARD) &&
> -(env->psw.mask & PSW_MASK_EXT)) {
> -cc->do_interrupt(cpu);
> -next_tb = 0;
> -}
> #endif
> /* The target hook has 3 exit conditions:
> False when the interrupt isn't processed,
> diff --git a/target-s390x/cpu-qom.h b/target-s390x/cpu-qom.h
> index 80dd741..4f7d4cb 100644
> --- a/target-s390x/cpu-qom.h
> +++ b/target-s390x/cpu-qom.h
> @@ -78,6 +78,7 @@ static inline S390CPU *s390_env_get_cpu(CPUS390XState *env)
> #define ENV_OFFSET offsetof(S390CPU, env)
>
> void s390_cpu_do_interrupt(CPUState *cpu);
> +bool s390_cpu_exec_interrupt(CPUState *cpu, int int_req);
> void s390_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function
> cpu_fprintf,
> int flags);
> int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 97a9216..2cfeb82 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -262,6 +262,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void
> *data)
> cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
> cc->write_elf64_note = s390_cpu_write_elf64_note;
> cc->write_elf64_qemunote = s390_cpu_write_elf64_qemunote;
> +cc->cpu_exec_interrupt = s390_cpu_exec_interrupt;
> #endif
> dc->vmsd = &vmstate_s390_cpu;
> cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index 67ab106..e21afe6 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -876,4 +876,17 @@ void s390_cpu_do_interrupt(CPUState *cs)
> }
> }
>
> +bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> +{
> +if (interrupt_request & CPU_INTERRUPT_HARD) {
> +S390CPU *cpu = S390_CPU(cs);
> +CPUS390XState *env = &cpu->env;
> +
> +if (env->psw.mask & PSW_MASK_EXT) {
> +s390_cpu_do_interrupt(cs);
> +return true;
> +}
> +}
> +return false;
> +}
> #endif /* CONFIG_USER_ONLY */
--
Alex Bennée
Re: [Qemu-devel] [PULL 0/2] sdl patch queue
On 15 September 2014 23:31, Gerd Hoffmann wrote: > Hi, > > Two minor sdl2 fixes. > > please pull, > Gerd > > The following changes since commit cc35a44cf7b522b1fd0b786562b7de4b881c41b0: > > Merge remote-tracking branch 'remotes/qmp-unstable/queue/qmp' into staging > (2014-09-15 19:44:34 +0100) > > are available in the git repository at: > > > git://git.kraxel.org/qemu tags/pull-sdl-20140916-1 > > for you to fetch changes up to 0d61f7dcc6c04e7e825417c74e50362d990330bf: > > sdl2: keymap fixups (2014-09-16 08:07:05 +0200) > > > Two minor sdl2 fixes. > > > Gerd Hoffmann (2): > sdl2: drop sdl_zoom.h > sdl2: keymap fixups Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Il 16/09/2014 18:56, Michael S. Tsirkin ha scritto: > On Tue, Sep 16, 2014 at 06:27:51PM +0200, Paolo Bonzini wrote: >> Il 16/09/2014 18:26, Michael S. Tsirkin ha scritto: >>> Right so types should be explicit. >>> If an arbitrary string isn't allowed, this should be documented. >>> It's not great as is: what's the format for macaddr? AA:BB:CC:DD:EE:FF? >>> aa:bb:cc:dd:ee:ff? aabbccddeeff? 0xaabbccddeeff? >>> But just saying "string" is going in the wrong direction imho. >> >> That's the purpose of documentation (docs/qdev-device-use.txt), > > That's not user documentation, that's developer documentation, > isn't it? It's user documentation. It's not distributed because we suck at documentation. >> and even >> then is better done with examples. I don't think doing it in -device >> foo,help (which I'm not even sure is particularly helpful. > > -device foo,help isn't helpful at all because it does not > tell people what does each option do. > But it really should be fixed. Exactly. >> I'm sympathetic towards fixing the drive->str change, but I have no idea >> how to do it. > > Change legacy_name to point to a detailed human-readable > description of the type? > E.g. "Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF"? If libvirt can cope with e1000.mac=str (Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF) that would work for me. > We really really should add description to all properties, too. This is a huge job. We have hundreds of properties. Paolo
Re: [Qemu-devel] [PATCH 07/23] target-xtensa: Use cpu_exec_interrupt qom hook
Richard Henderson writes:
> Cc: Max Filippov
> Signed-off-by: Richard Henderson
> @@ -669,12 +669,6 @@ int cpu_exec(CPUArchState *env)
> cc->do_interrupt(cpu);
> next_tb = 0;
> }
> -#elif defined(TARGET_XTENSA)
> -if (interrupt_request & CPU_INTERRUPT_HARD) {
> -cpu->exception_index = EXC_IRQ;
> -cc->do_interrupt(cpu);
> -next_tb = 0;
> -}
> #endif
>
> void xtensa_cpu_do_interrupt(CPUState *cpu);
> +bool xtensa_cpu_exec_interrupt(CPUState *cpu, int interrupt_request);
> cc->do_interrupt = xtensa_cpu_do_interrupt;
> +cc->cpu_exec_interrupt = xtensa_cpu_exec_interrupt;
>
> +bool xtensa_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> +{
> +if (interrupt_request & CPU_INTERRUPT_HARD) {
> +cs->exception_index = EXC_IRQ;
> +xtensa_cpu_do_interrupt(cs);
> +return true;
> +}
> +return false;
Do this mean when all this unwinding has occured cc->do_interrupt will
no longer be needed at all entries will go in via
$ARCH_cpu_exec_interrupt?
--
Alex Bennée
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
Il 16/09/2014 20:02, Richard Henderson ha scritto: > While we could probably fix this for ppc (using addis), it's not nearly so > easily fixable for arm -- without impacting performance anyway. > > Does 96k worth of TLBs really help that much? Are all 12 of them actually > used? Can we use a more complex encoding scheme for the mmu_idx and use less? In practice, only 3 to 7 are---hence my original attempt at using some kind of FIFO caching: user mode, translation enabled kernel mode, paging disabled kernel mode, paging enabled supervisor mode, paging disabled supervisor mode, paging enabled Plus perhaps kernel and supervisor mode with only data paging enabled. You could lump together the IR!=0, DR!=0 cases, and flush that one TLB index if the IR/DR pair changes with respect to the last time. This would use 6 indices. Paolo
[Qemu-devel] [PATCH v3 16/23] pc87312: Drop unused members of PC87312State
Signed-off-by: Markus Armbruster
---
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 v3 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.
Signed-off-by: Markus Armbruster
---
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 1d9a680..fac1211 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;
@@ -2041,7 +2039,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));
@@ -2058,7 +2055,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));
@@ -2097,7 +2093,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);
@@ -2111,105 +2106,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 = opaque;
-}
-
-static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
-{
-if (bs->dev_ops && bs->dev_ops->change_media_cb) {
-bool tray_was_closed = !bdrv_dev_is_tray_open(bs);
-bs->dev_ops->change_media_cb(bs->dev_opaque, load);
-if (tray_was_closed) {
-/* tray open */
-qapi_event_send_device_tra
[Qemu-devel] [PATCH v3 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
---
block-migration.c | 2 +-
block.c | 151 ++--
block/archipelago.c | 30 -
block/backup.c | 2 +-
block/blkdebug.c| 22 +++
block/blkverify.c | 20 +++---
block/commit.c | 2 +-
block/curl.c| 8 +--
block/iscsi.c | 8 +--
block/linux-aio.c | 8 +--
block/mirror.c | 6 +-
block/qed-gencb.c | 4 +-
block/qed-table.c | 10 +--
block/qed.c | 46 +++---
block/qed.h | 12 ++--
block/quorum.c | 38 +--
block/raw-aio.h | 8 +--
block/raw-posix.c | 32 +-
block/raw-win32.c | 16 ++---
block/raw_bsd.c | 8 +--
block/rbd.c | 58 -
block/sheepdog.c| 4 +-
block/stream.c | 2 +-
block/win32-aio.c | 8 +--
blockjob.c | 4 +-
dma-helpers.c | 24 +++
hw/block/nvme.h | 2 +-
hw/ide/ahci.c | 2 +-
hw/ide/ahci.h | 2 +-
hw/ide/core.c | 12 ++--
hw/ide/internal.h | 12 ++--
hw/ide/macio.c | 2 +-
hw/ide/pci.c| 2 +-
hw/ide/pci.h| 2 +-
hw/ppc/mac.h| 2 +-
hw/scsi/scsi-generic.c | 2 +-
include/block/aio.h | 12 ++--
include/block/block.h | 36 +--
include/block/block_int.h | 30 -
include/block/blockjob.h| 4 +-
include/block/thread-pool.h | 4 +-
include/hw/scsi/scsi.h | 2 +-
include/monitor/monitor.h | 4 +-
include/sysemu/dma.h| 26
monitor.c | 6 +-
tests/test-thread-pool.c| 2 +-
thread-pool.c | 8 +--
47 files changed, 353 insertions(+), 354 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 f68f06c..1d9a680 100644
--- a/block.c
+++ b/block.c
@@ -61,12 +61,12 @@ 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,
+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);
@@ -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,
+ BlockCompletionFunc *cb,
+ void *opaque,
+ bool is_write);
static void coroutine_fn bdrv_co_do_rw(void *opaque);
static int coroutine_fn bdrv
[Qemu-devel] [PATCH v3 21/23] blockdev: Convert qmp_eject(), qmp_change_blockdev() to BlockBackend
Much more command code needs conversion. I'm converting these now
because they's using bdrv_dev_* functions, which I'm about to lift
into BlockBackend.
Signed-off-by: Markus Armbruster
---
blockdev.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index e218c59..e115bde 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1502,8 +1502,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;
}
@@ -1527,15 +1529,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,
@@ -1594,16 +1596,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);
@@ -1613,7 +1617,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 v3 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
---
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 v3 18/23] blockdev: Fix blockdev-add not to create IDE drive (0, 0)
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. Not sure whether getting the QMP command
executed early enough is likely in practice, though.
Fix by creating DriveInfo in drive_new(). Block backends created by
blockdev-add don't get one.
A few places assume a block backend always has a DriveInfo. Fix them
up.
Signed-off-by: Markus Armbruster
---
block/block-backend.c | 4 +---
blockdev.c| 10 ++
hw/block/block.c | 16 ++--
hw/ide/qdev.c | 2 +-
hw/scsi/scsi-disk.c | 2 +-
5 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 141a31b..b55f0b4 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -101,9 +101,7 @@ static void drive_info_del(DriveInfo *dinfo)
if (!dinfo) {
return;
}
-if (dinfo->opts) {
-qemu_opts_del(dinfo->opts);
-}
+qemu_opts_del(dinfo->opts);
g_free(dinfo->serial);
g_free(dinfo);
}
diff --git a/blockdev.c b/blockdev.c
index 2def256..0d99be0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -285,7 +285,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 +456,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;
@@ -903,21 +899,19 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
}
/* Set legacy DriveInfo fields */
-dinfo = blk_legacy_dinfo(blk);
+dinfo = g_malloc0(sizeof(*dinfo));
dinfo->enable_auto_del = true;
dinfo->opts = all_opts;
-
dinfo->cyls = cyls;
dinfo->heads = heads;
dinfo->secs = secs;
dinfo->trans = translation;
-
dinfo->type = 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:
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) {
-*ptrans = dinfo->trans;
+if (dinfo) {
+conf->cyls = dinfo->cyls;
+conf->heads = dinfo->heads;
+conf->secs = dinfo->secs;
+if (ptrans) {
+*ptrans = dinfo->trans;
+}
}
}
if (!conf->cyls && !conf->heads && !conf->secs) {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 4818334..a74c81e 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -206,7 +206,7 @@ static int ide_drive_initfn(IDEDevice *dev)
{
DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
-return ide_dev_initfn(dev, dinfo->media_cd ? IDE_CD : IDE_HD);
+return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
}
#define DEFINE_IDE_DEV_PROPERTIES() \
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index b0bca48..9deef1a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2336,7 +2336,7 @@ static void scsi_disk_realize(SCSIDevice *dev, Error
**errp)
}
dinfo = blk_legacy_dinfo(dev->conf.blk);
-if (dinfo->media_cd) {
+if (dinfo && dinfo->media_cd) {
scsi_cd_realize(dev, errp);
} else {
scsi_hd_realize(dev, errp);
--
1.9.3
[Qemu-devel] [PATCH v3 15/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
---
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 f02ce4a..23964a2 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;
@@ -423,8 +424,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;
}
@@ -448,6 +449,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 v3 12/23] virtio-blk: Drop redundant VirtIOBlock member conf
Commit 12c5674 turned it into a pointer to member blk.conf.
Signed-off-by: Markus Armbruster
---
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 38ad38f..5943af5 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -298,7 +298,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);
@@ -519,19 +519,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
@@ -543,13 +544,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(&s->blk.conf);
blkcfg.alignment_offset = 0;
blkcfg.wce = bdrv_enable_write_cache(s->bs);
memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
@@ -756,9 +757,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;
@@ -777,11 +777,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 cf61154..18967c8 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 v3 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
---
block/block-backend.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index d49c988..5646628 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -253,6 +253,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);
@@ -281,9 +282,10 @@ void blk_detach_dev(BlockBackend *blk, void *dev)
/* TODO change to DeviceState *dev when all users are qdevified */
{
assert(blk->dev == dev);
-blk->dev = NULL;
blk->dev_ops = NULL;
blk->dev_opaque = NULL;
+blk->dev = NULL;
+blk_unref(blk);
bdrv_set_guest_block_size(blk->bs, 512);
qemu_coroutine_adjust_pool_size(-COROUTINE_POOL_RESERVATION);
}
--
1.9.3
[Qemu-devel] [PATCH v3 06/23] block: Make BlockBackend own its BlockDriverState
On BlockBackend destruction, unref its BlockDriverState. Replaces the
callers' unrefs.
Signed-off-by: Markus Armbruster
---
block/block-backend.c | 6 ++
blockdev.c| 8 ++--
hw/block/xen_disk.c | 6 +++---
qemu-img.c| 35 +--
qemu-io.c | 5 -
qemu-nbd.c| 1 -
6 files changed, 8 insertions(+), 53 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index be9acda..ed4873e 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() */
@@ -121,8 +121,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 1c97faa..3a6fd46 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);
@@ -1782,9 +1780,8 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
/* 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);
+blk_unref(blk);
}
aio_context_release(aio_context);
@@ -2521,7 +2518,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 206a513..40bd129 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;
@@ -1196,10 +1192,8 @@ static int img_compare(int argc, char **argv)
out:
qemu_vfree(buf1);
qemu_vfree
[Qemu-devel] [PATCH v3 13/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
---
hw/block/dataplane/virtio-blk.c | 33 +-
hw/block/dataplane/virtio-blk.h | 2 +-
hw/block/virtio-blk.c | 52 -
include/hw/virtio/virtio-blk.h | 2 +-
4 files changed, 45 insertions(+), 44 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->conf->conf.bs, qemu_get_aio_
[Qemu-devel] [PATCH v3 17/23] blockdev: Drop superfluous DriveInfo member id
Signed-off-by: Markus Armbruster
---
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 5f796b4..141a31b 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -104,7 +104,6 @@ static void drive_info_del(DriveInfo *dinfo)
if (dinfo->opts) {
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 e87bf4b..2def256 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 v3 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
---
block/Makefile.objs| 2 +-
block/block-backend.c | 120 +
blockdev.c | 11 +++-
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, 243 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 c9c8bbb..e410e7a 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 count.
+ * If this drops it t
[Qemu-devel] [PATCH v3 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
---
block.c | 2 --
block/block-backend.c | 38
blockdev.c| 73 ---
include/sysemu/blockdev.h | 4 +++
4 files changed, 79 insertions(+), 38 deletions(-)
diff --git a/block.c b/block.c
index 7ccf443..5f7dc45 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"
@@ -2124,7 +2123,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..9ee57c7 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;
+}
+}
+assert(0);
+}
+
+/*
* Hide @blk.
* @blk must not have been hidden already.
* Make attached BlockDriverState, if any, anonymous.
diff --git a/blockdev.c b/blockdev.c
index 5da6028..722d083 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;
+BlockBackend *blk
[Qemu-devel] [PATCH v3 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
---
block-migration.c | 12
block.c | 70 ---
block/block-backend.c | 14 ++
block/cow.c | 2 +-
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 | 4 +--
include/block/block_int.h | 2 --
18 files changed, 67 insertions(+), 75 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 50ccafd..c3fa3da 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"
@@ -334,30 +335,11 @@ 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;
+BlockDriverState *bs = bdrv_new();
-assert(*device_name);
-
-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();
-
-pstrcpy(bs->device_name, sizeof(bs->device_name), device_name);
QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
-
return bs;
}
@@ -1163,7 +1145,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;
@@ -1537,7 +1519,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;
@@ -1744,7 +1726,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);
+ bdr
[Qemu-devel] [PATCH v3 19/23] blockdev: Drop DriveInfo member enable_auto_del
Commit 2d246f0 introduced DriveInfo member enable_auto_del to
distinguish DriveInfo created via drive_new() from DriveInfo created
via qmp_blockdev_add(). The latter no longer exist. Drop
enable_auto_del.
Signed-off-by: Markus Armbruster
---
blockdev.c| 11 +++
include/sysemu/blockdev.h | 1 -
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 0d99be0..e218c59 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)
@@ -900,7 +898,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
/* Set legacy DriveInfo fields */
dinfo = g_malloc0(sizeof(*dinfo));
-dinfo->enable_auto_del = true;
dinfo->opts = all_opts;
dinfo->cyls = cyls;
dinfo->heads = heads;
@@ -1716,7 +1713,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;
@@ -1727,8 +1723,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/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 27a40d5..2129d81 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -35,7 +35,6 @@ struct DriveInfo {
int bus;
int unit;
int auto_del; /* see blockdev_mark_auto_del() */
-bool enable_auto_del; /* Only for legacy drive_new() */
int media_cd;
int cyls, heads, secs, trans;
QemuOpts *opts;
--
1.9.3
[Qemu-devel] [PATCH v3 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.
Signed-off-by: Markus Armbruster
Reviewed-by: Benoît Canet
---
block.c | 11 +++
block/block-backend.c | 13 ++---
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index c3fa3da..f68f06c 100644
--- a/block.c
+++ b/block.c
@@ -861,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;
@@ -3795,14 +3795,9 @@ void bdrv_iterate_format(void (*it)(void *opaque, const
char *name),
/* This function is to find block backend bs */
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 */
diff --git a/block/block-backend.c b/block/block-backend.c
index f14a291..7fd832d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -44,6 +44,12 @@ BlockBackend *blk_new(const char *name, Error **errp)
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);
@@ -61,13 +67,6 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp)
BlockBackend *blk;
BlockDriverState *bs;
-if (bdrv_find_node(name)) {
-error_setg(errp,
- "Device name '%s' conflicts with an existing node name",
- name);
-return NULL;
-}
-
blk = blk_new(name, errp);
if (!blk) {
return NULL;
--
1.9.3
[Qemu-devel] [PATCH v3 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 0/4] Miscellaneous block 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. 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_root() instead of bdrv_new_named(), because it'll lose its name parameter in PATCH 08. In v1, it gets removed there. - Drop a condition that cannot be false due to prior assertion * [PATCH 02/23] block: New BlockBackend - Commit message amended to explain when BlockBackends get created and destroyed [Kevin] - License reluctantly changed to LGPL2.1+ [Kevin] - Plug leak on error in blk_new() [Kevin] - g_assert() is silly, stick to plain assert() [Kevin] - Fix double-free when do_drive_del() leaves BDS destruction to blockdev_auto_del(), and user adds a BB with the same name in between [Kevin] Note: bug goes away later in v1 - Plug a bunch of leaks on error in qemu-img.c [Kevin, Benoît] Note: bugs goes away later in v1 - Update for "[PATCH 3/4] qemu-nbd: Destroy the BlockDriverState properly" * [PATCH 03/23] block: Connect BlockBackend to BlockDriverState - Keep the double-free just mentioned fixed; commit message amended to explain the drive_del complications Basically move v1's solution from "[PATCH 14/23] hw: Convert from BlockDriverState to BlockBackend, mostly" here, plus a fix for invalid queue removal in blk_delete() - Cleanup extracted and posted separately as "[PATCH 1/4] blockdev: Disentangle BlockDriverState and DriveInfo creation" [Kevin] - Inline blk_attach_bs(), blk_detach_bs() into only callers - Rebase on PATCH 02's qemu-img.c leak fixes, basically reverting its error path complications - Update for "[PATCH 3/4] qemu-nbd: Destroy the BlockDriverState properly" * [PATCH 04/23] block: Connect BlockBackend and DriveInfo - Bug fix extracted and posted separately as "[PATCH 2/4] block: Keep DriveInfo alive until BlockDriverState dies" [Kevin] Unlike v1, it takes care to preserve the guard against qemu_opts_del(NULL) Could only happen when drive_del'ing something created with blockdev-add, which is impossible since "[PATCH] blockdev: Refuse to drive_del something added with blockdev-add", but not obviously so Becomes obvious in PATCH 18, which duly drops the guard - Plug leak on error in drive_new() [Kevin] * [PATCH 05/23] block: Code motion to get rid of stubs/blockdev.c - New; to separate some of the cleanup of the mess made in said extracted bug fix from the previous patch, to keep it more readable * [PATCH 06/23] block: Make BlockBackend own its BlockDriverState - Update for "[PATCH 3/4] qemu-nbd: Destroy the BlockDriverState properly" * [PATCH 07/23] block: Eliminate bdrv_states, use block_next() instead - Drop; it can break things when a root BDS outlives its BB, because something else is holding a reference to the BDS when the BB dies * [PATCH 07/23] block: Eliminate bdrv_iterate(), use bdrv_next() - Fix accidental removal of a line [Benoît] * [PATCH 08/23] block: Eliminate BlockDriverState member device_name[] - Commit message: typo fixed [Eric], clarified - Update for [PATCH 4/4] block: Improve message for device name clashing with node name - Ripple effects from drop of P
[Qemu-devel] [PATCH v3 07/23] block: Eliminate bdrv_iterate(), use bdrv_next()
Signed-off-by: Markus Armbruster
Reviewed-by: Benoît Canet
---
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 5f7dc45..50ccafd 100644
--- a/block.c
+++ b/block.c
@@ -3899,15 +3899,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 3a6fd46..85f574b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2527,26 +2527,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 8cf953a..02cf956 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -410,8 +410,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 667efb7..7e450ec 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4208,24 +4208,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->rs, name);
-}
-}
-
static const char *next_a
[Qemu-devel] [PATCH v3 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
---
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 | 9 ++---
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, 166 insertions(+), 94 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 85f574b..49496b6 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/devices.h"
#include "hw/boards.h"
+#include "sysemu/block-backend.h"
#include
[Qemu-devel] [PATCH v3 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
---
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 2ac6a3b..934881f 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 (bdrv_find(device_name)) {
error_setg(errp, "Device with id '%s' already exists",
@@ -353,12 +354,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]);
}
@@ -1219,7 +1229,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);
@@ -1348,7 +1358,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);
@@ -1419,7 +1429,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 84bcae8..166d1b9 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1528,7 +1528,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 450f95c..c9463e3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -463,7 +463,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 07d6d8e..8cf953a 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(const char *device_name, Error **
[Qemu-devel] [PATCH v3 05/23] block: Code motion to get rid of stubs/blockdev.c
Signed-off-by: Markus Armbruster
Reviewed-by: Benoît Canet
---
block/block-backend.c | 15 +++
blockdev.c| 13 -
include/sysemu/blockdev.h | 1 -
stubs/Makefile.objs | 1 -
stubs/blockdev.c | 12
5 files changed, 15 insertions(+), 27 deletions(-)
delete mode 100644 stubs/blockdev.c
diff --git a/block/block-backend.c b/block/block-backend.c
index 9ee57c7..be9acda 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,19 @@ static void blk_delete(BlockBackend *blk)
g_free(blk);
}
+static void drive_info_del(DriveInfo *dinfo)
+{
+if (!dinfo) {
+return;
+}
+if (dinfo->opts) {
+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 722d083..1c97faa 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -223,19 +223,6 @@ void drive_del(DriveInfo *dinfo)
blk_unref(blk_by_legacy_dinfo(dinfo));
}
-void drive_info_del(DriveInfo *dinfo)
-{
-if (!dinfo) {
-return;
-}
-if (dinfo->opts) {
-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 v3 03/23] block: Connect BlockBackend to BlockDriverState
The pointer from BlockBackend to BlockDriverState is a strong
reference, managed with bdrv_ref() / bdrv_unref(), the back-pointer is
a weak one.
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 in a few commits, so I mark it
FIXME in the code now, and plug it when it's easy.
Signed-off-by: Markus Armbruster
---
block.c| 10 ++--
block/block-backend.c | 71 ++-
blockdev.c | 21 ---
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(+), 94 deletions(-)
diff --git a/block.c b/block.c
index 934881f..7ccf443 100644
--- a/block.c
+++ b/block.c
@@ -2032,7 +2032,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.
*/
@@ -2051,8 +2051,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);
@@ -2068,8 +2069,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);
@@ -2096,7 +2098,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) {
+blk->bs->blk = NULL;
+blk->bs = NULL;
+}
+/* Avoid double-remove after blk_hide_on_behalf_of_do_drive_del() */
+if (blk->name[0]) {
+QTAILQ_REMOVE(&blk_backends, blk, link);
+}
g_free(blk->name);
Re: [Qemu-devel] [PATCH] kvmclock: clarify usage of cpu_clean_all_dirty
On Tue, Sep 16, 2014 at 06:22:15PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 18:07, Marcelo Tosatti ha scritto: > >> > The cpu_synchronize_all_states() call in kvmclock_vm_state_change() is > >> > needed to make env->tsc up to date with the value on the source, right? > > Its there to make sure the pair > > > > env->tsc, s->clock = data.clock > > > > are relative to point close in time. > > Ok. But why are they not close in time? > > Could we have the opposite situation where env->tsc is loaded a long > time _after_ s->clock, and something breaks? > > Also, there is no reason to do kvmclock_current_nsec() during normal > execution of the VM. Is the s->clock sent by the source ever good > across migration, and could the source send kvmclock_current_nsec() > instead of whatever KVM_GET_CLOCK returns? guest clock read = pvclock.system_timestamp + (rdtsc() - pvclock.tsc) Host kernel updates pvclock.system_timestamp in certain situations, such as guest initialization. With master clock scheme, pvclock.system_timestamp is only updated on guest initialization. In case TSC runs faster than the host system clock, you cannot do the following on destination: pvclock.system_timestamp = ioctl(KVM_GET_CLOCK) povclock.tsc = rdtsc guest clock read = pvclock.system_timestampOLD + (rdtsc() - pvclock.tsc) Because the effective clock was not pvclock.system_timestamp but the TSC, which is running at a higher frequency. If you do that, the time goes backward. Q: could the source send kvmclock_current_nsec() instead of whatever KVM_GET_CLOCK returns? Well no because there are other users of KVM_GET_CLOCK such as Hyper-V clock. > I don't understand this code very well, but it seems to me that the > migration handling and VM state change handler are mixed up... Again, suggestions are welcome. > > Paolo > > >> > But if the synchronize_all_states+clean_all_dirty pair runs on the > >> > source, why is the cpu_synchronize_all_states() call in > >> > qemu_savevm_state_complete() not enough? It runs even later than > >> > kvmclock_vm_state_change. > > Because of the "pair of time values" explanation above. >
Re: [Qemu-devel] [PATCH 06/23] qom: Add cpu_exec_interrupt hook
Richard Henderson writes:
> Continuing the removal of ifdefs from cpu_exec.
>
> Cc: Andreas Färber
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
> ---
> cpu-exec.c| 14 +-
> include/qom/cpu.h | 2 ++
> qom/cpu.c | 6 ++
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index d930e7a..fe313b4 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -676,8 +676,15 @@ int cpu_exec(CPUArchState *env)
> next_tb = 0;
> }
> #endif
> - /* Don't use the cached interrupt_request value,
> - do_interrupt may have updated the EXITTB flag. */
> +/* The target hook has 3 exit conditions:
> + False when the interrupt isn't processed,
> + True when it is, and we should restart on a new TB,
> + and via longjmp via cpu_loop_exit. */
> +if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
> +next_tb = 0;
> +}
> +/* Don't use the cached interrupt_request value,
> + do_interrupt may have updated the EXITTB flag. */
> if (cpu->interrupt_request & CPU_INTERRUPT_EXITTB) {
> cpu->interrupt_request &= ~CPU_INTERRUPT_EXITTB;
> /* ensure that no TB jump will be modified as
> @@ -783,10 +790,7 @@ int cpu_exec(CPUArchState *env)
> * local variables as longjmp is marked 'noreturn'. */
> cpu = current_cpu;
> env = cpu->env_ptr;
> -#if !(defined(CONFIG_USER_ONLY) && \
> - (defined(TARGET_M68K) || defined(TARGET_PPC) || defined(TARGET_S390X)))
> cc = CPU_GET_CLASS(cpu);
> -#endif
> #ifdef TARGET_I386
> x86_cpu = X86_CPU(cpu);
> #endif
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 0340cf4..f576b47 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -101,6 +101,7 @@ struct TranslationBlock;
> * @gdb_core_xml_file: File name for core registers GDB XML description.
> * @cpu_exec_enter: Callback for cpu_exec preparation.
> * @cpu_exec_exit: Callback for cpu_exec cleanup.
> + * @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec.
> *
> * Represents a CPU family or model.
> */
> @@ -154,6 +155,7 @@ typedef struct CPUClass {
>
> void (*cpu_exec_enter)(CPUState *cpu);
> void (*cpu_exec_exit)(CPUState *cpu);
> +bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request);
> } CPUClass;
>
> #ifdef HOST_WORDS_BIGENDIAN
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 6a9d02e..0ec3337 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -206,6 +206,11 @@ static void cpu_common_noop(CPUState *cpu)
> {
> }
>
> +static bool cpu_common_exec_interrupt(CPUState *cpu, int int_req)
> +{
> +return false;
> +}
> +
> void cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
> int flags)
> {
> @@ -347,6 +352,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
> k->debug_excp_handler = cpu_common_noop;
> k->cpu_exec_enter = cpu_common_noop;
> k->cpu_exec_exit = cpu_common_noop;
> +k->cpu_exec_interrupt = cpu_common_exec_interrupt;
> dc->realize = cpu_common_realizefn;
> /*
> * Reason: CPUs still need special care by board code: wiring up
--
Alex Bennée
Re: [Qemu-devel] [PATCH 2/2] tcg: Always enable TCGv type checking
On 16 September 2014 10:46, Richard Henderson wrote:
> Instead of using structures, which imply some amount of overhead
> on certain ABIs, use pointer types.
>
> This actually reduces the size of the binaries vs a NON-debug
> build on ppc64 and x86_64, due to a reduction in the number of
> sign-extension insns.
It's good to have this typechecking enabled unconditionally.
> Signed-off-by: Richard Henderson
> ---
> tcg/tcg.h | 89
> ---
> 1 file changed, 34 insertions(+), 55 deletions(-)
>
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 997a704..7285f71 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -274,75 +274,54 @@ typedef enum TCGMemOp {
>
> typedef tcg_target_ulong TCGArg;
>
> -/* Define a type and accessor macros for variables. Using a struct is
> - nice because it gives some level of type safely. Ideally the compiler
> - be able to see through all this. However in practice this is not true,
> - especially on targets with braindamaged ABIs (e.g. i386).
> - We use plain int by default to avoid this runtime overhead.
> - Users of tcg_gen_* don't need to know about any of this, and should
> - treat TCGv as an opaque type.
> +/* Define a type and accessor macros for variables. Using pointer types
> + is nice because it gives some level of type safely. Converting to and
"safety".
Otherwise
Reviewed-by: Peter Maydell
-- PMM
Re: [Qemu-devel] [PATCH 1/2] qemu/compiler: Define QEMU_ARTIFICIAL
On 16 September 2014 10:46, Richard Henderson wrote: > The combination of always_inline + artificial allows tiny inline > functions to be written that do not interfere with debugging. > In particular, gdb will not step into an artificial function. > > The always_inline attribute was introduced in gcc 4.2, > and the artificial attribute was introduced in gcc 4.3. > > Signed-off-by: Richard Henderson clang can't handle the "artificial" attribute but it reports itself as gcc 4.2 so that's OK. Reviewed-by: Peter Maydell -- PMM
[Qemu-devel] [PATCH v3] async: aio_context_new(): Handle event_notifier_init failure
If event_notifier_init fails QEMU exits without printing
any error information to the user. This commit adds an error
message on failure:
# qemu [...]
qemu: Failed to initialize event notifier: Too many open files in system
Signed-off-by: Chrysostomos Nanakos
---
async.c | 16 +++-
include/block/aio.h |2 +-
include/qemu/main-loop.h |2 +-
iothread.c | 11 ++-
main-loop.c |9 +++--
qemu-img.c |8 +++-
qemu-io.c|7 ++-
qemu-nbd.c |6 +-
tests/test-aio.c | 10 +-
tests/test-thread-pool.c | 10 +-
tests/test-throttle.c| 10 +-
vl.c |5 +++--
12 files changed, 78 insertions(+), 18 deletions(-)
diff --git a/async.c b/async.c
index a99e7f6..6e1b282 100644
--- a/async.c
+++ b/async.c
@@ -289,18 +289,24 @@ static void aio_rfifolock_cb(void *opaque)
aio_notify(opaque);
}
-AioContext *aio_context_new(void)
+AioContext *aio_context_new(Error **errp)
{
+int ret;
AioContext *ctx;
ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
+ret = event_notifier_init(&ctx->notifier, false);
+if (ret < 0) {
+g_source_destroy(&ctx->source);
+error_setg_errno(errp, -ret, "Failed to initialize event notifier");
+return NULL;
+}
+aio_set_event_notifier(ctx, &ctx->notifier,
+ (EventNotifierHandler *)
+ event_notifier_test_and_clear);
ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
ctx->thread_pool = NULL;
qemu_mutex_init(&ctx->bh_lock);
rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx);
-event_notifier_init(&ctx->notifier, false);
-aio_set_event_notifier(ctx, &ctx->notifier,
- (EventNotifierHandler *)
- event_notifier_test_and_clear);
timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);
return ctx;
diff --git a/include/block/aio.h b/include/block/aio.h
index 4603c0f..f3d5517 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -99,7 +99,7 @@ void aio_set_dispatching(AioContext *ctx, bool dispatching);
* They also provide bottom halves, a service to execute a piece of code
* as soon as possible.
*/
-AioContext *aio_context_new(void);
+AioContext *aio_context_new(Error **errp);
/**
* aio_context_ref:
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 6f0200a..62c68c0 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -42,7 +42,7 @@
*
* In the case of QEMU tools, this will also start/initialize timers.
*/
-int qemu_init_main_loop(void);
+int qemu_init_main_loop(Error **errp);
/**
* main_loop_wait: Run one iteration of the main loop.
diff --git a/iothread.c b/iothread.c
index d9403cf..6e394a0 100644
--- a/iothread.c
+++ b/iothread.c
@@ -17,6 +17,7 @@
#include "block/aio.h"
#include "sysemu/iothread.h"
#include "qmp-commands.h"
+#include "qemu/error-report.h"
#define IOTHREADS_PATH "/objects"
@@ -53,6 +54,9 @@ static void iothread_instance_finalize(Object *obj)
{
IOThread *iothread = IOTHREAD(obj);
+if (!iothread->ctx) {
+return;
+}
iothread->stopping = true;
aio_notify(iothread->ctx);
qemu_thread_join(&iothread->thread);
@@ -63,10 +67,15 @@ static void iothread_instance_finalize(Object *obj)
static void iothread_complete(UserCreatable *obj, Error **errp)
{
+Error *local_error = NULL;
IOThread *iothread = IOTHREAD(obj);
iothread->stopping = false;
-iothread->ctx = aio_context_new();
+iothread->ctx = aio_context_new(&local_error);
+if (!iothread->ctx) {
+error_propagate(errp, local_error);
+return;
+}
iothread->thread_id = -1;
qemu_mutex_init(&iothread->init_done_lock);
diff --git a/main-loop.c b/main-loop.c
index 3cc79f8..2f83d80 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -126,10 +126,11 @@ void qemu_notify_event(void)
static GArray *gpollfds;
-int qemu_init_main_loop(void)
+int qemu_init_main_loop(Error **errp)
{
int ret;
GSource *src;
+Error *local_error = NULL;
init_clocks();
@@ -138,8 +139,12 @@ int qemu_init_main_loop(void)
return ret;
}
+qemu_aio_context = aio_context_new(&local_error);
+if (!qemu_aio_context) {
+error_propagate(errp, local_error);
+return -1;
+}
gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
-qemu_aio_context = aio_context_new();
src = aio_get_g_source(qemu_aio_context);
g_source_attach(src, NULL);
g_source_unref(src);
diff --git a/qemu-img.c b/qemu-img.c
index 91d1ac3..dbf0904 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2879,6 +2879,7 @@ int main(int argc, char **argv)
{
const img_cmd_t *cmd;
const char *cmdname;
+Error *local_error = NULL;
[Qemu-devel] [PATCH v3] async: aio_context_new(): Handle event_notifier_init failure
v2->v3 -- * Remove errno usage and print the detailed message based on errno when event_notifier_init() fails. * Propagate error and return from iothread_complete() if aio_context_new() fails. * Return if !iothread->ctx from iothread_instance_finalize(), used by QOM when object_unref(obj) is called after user_creatable_complete() fails. * Remove cosmetic fixes accidentally introduced by editor and fix code style issues. v1->v2 -- * aio_context_new() returns NULL if the initialization of event notifier fails. * Add descriptive error messages if aio_context_new() and event_notifier_init() fail. * Fix gpollfds leak. Chrysostomos Nanakos (1): async: aio_context_new(): Handle event_notifier_init failure async.c | 16 +++- include/block/aio.h |2 +- include/qemu/main-loop.h |2 +- iothread.c | 11 ++- main-loop.c |9 +++-- qemu-img.c |8 +++- qemu-io.c|7 ++- qemu-nbd.c |6 +- tests/test-aio.c | 10 +- tests/test-thread-pool.c | 10 +- tests/test-throttle.c| 10 +- vl.c |5 +++-- 12 files changed, 78 insertions(+), 18 deletions(-) -- 1.7.10.4
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
On 09/16/2014 10:20 AM, Tom Musta wrote:
> On 9/15/2014 10:03 AM, Paolo Bonzini wrote:
>> PowerPC TCG flushes the TLB on every IR/DR change, which basically
>> means on every user<->kernel context switch. Encode IR/DR in the
>> MMU index.
>>
>> This brings the number of TLB flushes down from ~90 to ~5
>> for starting up the Debian installer, which is in line with x86
>> and gives a ~10% performance improvement.
>>
>> Signed-off-by: Paolo Bonzini
>> ---
>> target-ppc/cpu.h | 7 ++-
>> target-ppc/excp_helper.c | 3 ---
>> target-ppc/helper_regs.h | 11 ++-
>> 3 files changed, 8 insertions(+), 13 deletions(-)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index b64c652..c29ce3b 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -922,7 +922,8 @@ struct ppc_segment_page_sizes {
>>
>>
>> /*/
>> /* The whole PowerPC CPU context */
>> -#define NB_MMU_MODES 3
>> +#define NB_MMU_MODES 12
>> +#define MMU_USER_IDX 3 /* PR=IR=DR=1 */
>
> This doesn't build for me:
>
> CCppc64-softmmu/tcg/tcg.o
> In file included from /bghome/tmusta/powerisa/qemu/qemu/tcg/tcg.c:264:
> /bghome/tmusta/powerisa/qemu/qemu/tcg/ppc/tcg-target.c: In function
> ?tcg_out_tlb_read?:
> /bghome/tmusta/powerisa/qemu/qemu/tcg/ppc/tcg-target.c:1394: error: size of
> array ?qemu_build_bug_on__1396? is negative
> make[1]: *** [tcg/tcg.o] Error 1
> make: *** [subdir-ppc64-softmmu] Error 2
>
> which correlates with this:
>
> 1389 /* Compensate for very large offsets. */
> 1390 if (add_off >= 0x8000) {
> 1391 /* Most target env are smaller than 32k; none are larger than
> 64k.
> 1392 Simplify the logic here merely to offset by 0x7ff0, giving
> us a
> 1393 range just shy of 64k. Check this assumption. */
> 1394 QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
> 1395 tlb_table[NB_MMU_MODES - 1][1])
> 1396> 0x7ff0 + 0x7fff);
> 1397 tcg_out32(s, ADDI | TAI(TCG_REG_TMP1, base, 0x7ff0));
> 1398 base = TCG_REG_TMP1;
> 1399 cmp_off -= 0x7ff0;
> 1400 add_off -= 0x7ff0;
> 1401 }
Ouch, yes indeed.
While we could probably fix this for ppc (using addis), it's not nearly so
easily fixable for arm -- without impacting performance anyway.
Does 96k worth of TLBs really help that much? Are all 12 of them actually
used? Can we use a more complex encoding scheme for the mmu_idx and use less?
r~
Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure
On Tue, Sep 16, 2014 at 06:50:38PM +0300, Chrysostomos Nanakos wrote: > On Tue, Sep 16, 2014 at 05:45:16PM +0200, Paolo Bonzini wrote: > > Il 16/09/2014 17:43, Chrysostomos Nanakos ha scritto: > > >> > error_propagate(errp, local_error); > > >> > return; > > > Just to note that after propagating the error and returning, QEMU fails > > > silently without printing the error message. > > > > What is your testcase? > > I am starting QEMU with the options below and explicitly set iothread->ctx to > NULL. Is that ok as a testcase or should I reduce my open files limit to > produce the error? > > qemu --enable-kvm -smp 2 -m 1024 -object iothread,id=iothread0 -drive > file=archipelago:fedora_stable > -vnc 0.0.0.0:0 -qmp tcp:127.0.0.1:,server,nowait > > No error message, at least the propagated one, fails silently. > The segfault is caused when QOM tries to object_unref() the iothread object, type->instance_finalize(obj) is called which calls the registered iothread_instance_finalize function. A check there for the iothread->ctx seems to solve the problem. I will include the fix in the next patch series. Regards, Chrysostomos.
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On Tue, Sep 16, 2014 at 10:23:03AM -0700, Peter Maydell wrote: > On 16 September 2014 09:57, Michael S. Tsirkin wrote: > > Could you pls tell me whether my patch works, in your testing? > > My box which had such an old glib seems to be dead. > > Can you give me a Subject: line or a URL in patchwork, > please? (I can't conveniently search by message-id.) > > thanks > -- PMM Gmane supports lookups by ID. This is it: http://mid.gmane.org/20140916144324.ga7...@redhat.com To get raw mailbox, add /raw : http://article.gmane.org/gmane.comp.emulators.qemu/297262/raw OK? -- MST
[Qemu-devel] [PATCH 0/2] Always enable TCGv type checking
We've had a number of bugs slip through recently that would have been caught by --enable-tcg-debug. r~ Richard Henderson (2): qemu/compiler: Define QEMU_ARTIFICIAL tcg: Always enable TCGv type checking include/qemu/compiler.h | 6 tcg/tcg.h | 89 +++-- 2 files changed, 40 insertions(+), 55 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 2/2] tcg: Always enable TCGv type checking
Instead of using structures, which imply some amount of overhead
on certain ABIs, use pointer types.
This actually reduces the size of the binaries vs a NON-debug
build on ppc64 and x86_64, due to a reduction in the number of
sign-extension insns.
Signed-off-by: Richard Henderson
---
tcg/tcg.h | 89 ---
1 file changed, 34 insertions(+), 55 deletions(-)
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 997a704..7285f71 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -274,75 +274,54 @@ typedef enum TCGMemOp {
typedef tcg_target_ulong TCGArg;
-/* Define a type and accessor macros for variables. Using a struct is
- nice because it gives some level of type safely. Ideally the compiler
- be able to see through all this. However in practice this is not true,
- especially on targets with braindamaged ABIs (e.g. i386).
- We use plain int by default to avoid this runtime overhead.
- Users of tcg_gen_* don't need to know about any of this, and should
- treat TCGv as an opaque type.
+/* Define a type and accessor macros for variables. Using pointer types
+ is nice because it gives some level of type safely. Converting to and
+ from intptr_t rather than int reduces the number of sign-extension
+ instructions that get implied on 64-bit hosts. Users of tcg_gen_* don't
+ need to know about any of this, and should treat TCGv as an opaque type.
In addition we do typechecking for different types of variables. TCGv_i32
and TCGv_i64 are 32/64-bit variables respectively. TCGv and TCGv_ptr
- are aliases for target_ulong and host pointer sized values respectively.
- */
+ are aliases for target_ulong and host pointer sized values respectively. */
-#ifdef CONFIG_DEBUG_TCG
-#define DEBUG_TCGV 1
-#endif
+typedef struct TCGv_i32_d *TCGv_i32;
+typedef struct TCGv_i64_d *TCGv_i64;
+typedef struct TCGv_ptr_d *TCGv_ptr;
-#ifdef DEBUG_TCGV
+static inline TCGv_i32 QEMU_ARTIFICIAL MAKE_TCGV_I32(intptr_t i)
+{
+return (TCGv_i32)i;
+}
-typedef struct
+static inline TCGv_i64 QEMU_ARTIFICIAL MAKE_TCGV_I64(intptr_t i)
{
-int i32;
-} TCGv_i32;
+return (TCGv_i64)i;
+}
-typedef struct
+static inline TCGv_ptr QEMU_ARTIFICIAL MAKE_TCGV_PTR(intptr_t i)
{
-int i64;
-} TCGv_i64;
-
-typedef struct {
-int iptr;
-} TCGv_ptr;
-
-#define MAKE_TCGV_I32(i) __extension__ \
-({ TCGv_i32 make_tcgv_tmp = {i}; make_tcgv_tmp;})
-#define MAKE_TCGV_I64(i) __extension__ \
-({ TCGv_i64 make_tcgv_tmp = {i}; make_tcgv_tmp;})
-#define MAKE_TCGV_PTR(i) __extension__ \
-({ TCGv_ptr make_tcgv_tmp = {i}; make_tcgv_tmp; })
-#define GET_TCGV_I32(t) ((t).i32)
-#define GET_TCGV_I64(t) ((t).i64)
-#define GET_TCGV_PTR(t) ((t).iptr)
-#if TCG_TARGET_REG_BITS == 32
-#define TCGV_LOW(t) MAKE_TCGV_I32(GET_TCGV_I64(t))
-#define TCGV_HIGH(t) MAKE_TCGV_I32(GET_TCGV_I64(t) + 1)
-#endif
+return (TCGv_ptr)i;
+}
+
+static inline intptr_t QEMU_ARTIFICIAL GET_TCGV_I32(TCGv_i32 t)
+{
+return (intptr_t)t;
+}
-#else /* !DEBUG_TCGV */
+static inline intptr_t QEMU_ARTIFICIAL GET_TCGV_I64(TCGv_i64 t)
+{
+return (intptr_t)t;
+}
-typedef int TCGv_i32;
-typedef int TCGv_i64;
-#if TCG_TARGET_REG_BITS == 32
-#define TCGv_ptr TCGv_i32
-#else
-#define TCGv_ptr TCGv_i64
-#endif
-#define MAKE_TCGV_I32(x) (x)
-#define MAKE_TCGV_I64(x) (x)
-#define MAKE_TCGV_PTR(x) (x)
-#define GET_TCGV_I32(t) (t)
-#define GET_TCGV_I64(t) (t)
-#define GET_TCGV_PTR(t) (t)
+static inline intptr_t QEMU_ARTIFICIAL GET_TCGV_PTR(TCGv_ptr t)
+{
+return (intptr_t)t;
+}
#if TCG_TARGET_REG_BITS == 32
-#define TCGV_LOW(t) (t)
-#define TCGV_HIGH(t) ((t) + 1)
+#define TCGV_LOW(t) MAKE_TCGV_I32(GET_TCGV_I64(t))
+#define TCGV_HIGH(t) MAKE_TCGV_I32(GET_TCGV_I64(t) + 1)
#endif
-#endif /* DEBUG_TCGV */
-
#define TCGV_EQUAL_I32(a, b) (GET_TCGV_I32(a) == GET_TCGV_I32(b))
#define TCGV_EQUAL_I64(a, b) (GET_TCGV_I64(a) == GET_TCGV_I64(b))
#define TCGV_EQUAL_PTR(a, b) (GET_TCGV_PTR(a) == GET_TCGV_PTR(b))
--
1.9.3
[Qemu-devel] [PATCH 1/2] qemu/compiler: Define QEMU_ARTIFICIAL
The combination of always_inline + artificial allows tiny inline functions to be written that do not interfere with debugging. In particular, gdb will not step into an artificial function. The always_inline attribute was introduced in gcc 4.2, and the artificial attribute was introduced in gcc 4.3. Signed-off-by: Richard Henderson --- include/qemu/compiler.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index 155b358..ac7c4c4 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -24,6 +24,12 @@ #define QEMU_WARN_UNUSED_RESULT #endif +#if QEMU_GNUC_PREREQ(4, 3) +#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial)) +#else +#define QEMU_ARTIFICIAL +#endif + #if defined(_WIN32) # define QEMU_PACKED __attribute__((gcc_struct, packed)) #else -- 1.9.3
Re: [Qemu-devel] [PATCH v2 08/23] block: Eliminate BlockDriverState member device_name[]
Paolo Bonzini writes:
> Il 16/09/2014 16:08, Markus Armbruster ha scritto:
>> +if (bs->device_list.tqe_prev) {
>> QTAILQ_REMOVE(&bdrv_states, bs, device_list);
>> +bs->device_list.tqe_prev = NULL;
>>> >
>>> > I think a comments explaining the trick you are doing here would be
>>> > worthy:
>>> > after all you are touching directly the inner parts of a linked list and
>>> > bypassing the list API.
>> Fair enough.
>
> Or just add tqe_prev = NULL to QTAILQ_REMOVE.
And add a QTAILQ_IN_ANY_QUEUE() macro, so I get rid of both .tqe_prev
uses. I'll consider it, but either in v4 or as a follow-up patch,
because I want to grep the source for similar trickery.
Re: [Qemu-devel] [PULL 0/1] spice: call qemu_spice_set_passwd() during init
On 15 September 2014 23:12, Gerd Hoffmann wrote: > Hi, > > Spice patch queue, with a single patch this time. > > please pull, > Gerd > > The following changes since commit cc35a44cf7b522b1fd0b786562b7de4b881c41b0: > > Merge remote-tracking branch 'remotes/qmp-unstable/queue/qmp' into staging > (2014-09-15 19:44:34 +0100) > > are available in the git repository at: > > > git://anongit.freedesktop.org/spice/qemu tags/pull-spice-20140916-2 > > for you to fetch changes up to 07d49a53b6394941ed833486a3acb5c480d87db2: > > spice: call qemu_spice_set_passwd() during init (2014-09-16 08:09:03 +0200) > > > spice: call qemu_spice_set_passwd() during init > > Applied, thanks. -- PMM
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On 16 September 2014 09:57, Michael S. Tsirkin wrote: > Could you pls tell me whether my patch works, in your testing? > My box which had such an old glib seems to be dead. Can you give me a Subject: line or a URL in patchwork, please? (I can't conveniently search by message-id.) thanks -- PMM
Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes
On 9/15/2014 10:03 AM, Paolo Bonzini wrote:
> PowerPC TCG flushes the TLB on every IR/DR change, which basically
> means on every user<->kernel context switch. Encode IR/DR in the
> MMU index.
>
> This brings the number of TLB flushes down from ~90 to ~5
> for starting up the Debian installer, which is in line with x86
> and gives a ~10% performance improvement.
>
> Signed-off-by: Paolo Bonzini
> ---
> target-ppc/cpu.h | 7 ++-
> target-ppc/excp_helper.c | 3 ---
> target-ppc/helper_regs.h | 11 ++-
> 3 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index b64c652..c29ce3b 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -922,7 +922,8 @@ struct ppc_segment_page_sizes {
>
>
> /*/
> /* The whole PowerPC CPU context */
> -#define NB_MMU_MODES 3
> +#define NB_MMU_MODES 12
> +#define MMU_USER_IDX 3 /* PR=IR=DR=1 */
This doesn't build for me:
CCppc64-softmmu/tcg/tcg.o
In file included from /bghome/tmusta/powerisa/qemu/qemu/tcg/tcg.c:264:
/bghome/tmusta/powerisa/qemu/qemu/tcg/ppc/tcg-target.c: In function
?tcg_out_tlb_read?:
/bghome/tmusta/powerisa/qemu/qemu/tcg/ppc/tcg-target.c:1394: error: size of
array ?qemu_build_bug_on__1396? is negative
make[1]: *** [tcg/tcg.o] Error 1
make: *** [subdir-ppc64-softmmu] Error 2
which correlates with this:
1389 /* Compensate for very large offsets. */
1390 if (add_off >= 0x8000) {
1391 /* Most target env are smaller than 32k; none are larger than
64k.
1392 Simplify the logic here merely to offset by 0x7ff0, giving
us a
1393 range just shy of 64k. Check this assumption. */
1394 QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
1395 tlb_table[NB_MMU_MODES - 1][1])
1396> 0x7ff0 + 0x7fff);
1397 tcg_out32(s, ADDI | TAI(TCG_REG_TMP1, base, 0x7ff0));
1398 base = TCG_REG_TMP1;
1399 cmp_off -= 0x7ff0;
1400 add_off -= 0x7ff0;
1401 }
>
> #define PPC_CPU_OPCODES_LEN 0x40
>
> @@ -1231,10 +1232,6 @@ static inline CPUPPCState *cpu_init(const char
> *cpu_model)
> #define cpu_list ppc_cpu_list
>
> /* MMU modes definitions */
> -#define MMU_MODE0_SUFFIX _user
> -#define MMU_MODE1_SUFFIX _kernel
> -#define MMU_MODE2_SUFFIX _hypv
> -#define MMU_USER_IDX 0
> static inline int cpu_mmu_index (CPUPPCState *env)
> {
> return env->mmu_idx;
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index 922e86d..96ad9d7 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -623,9 +623,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int
> excp_model, int excp)
>
> if (env->spr[SPR_LPCR] & LPCR_AIL) {
> new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
> -} else if (msr & ((1 << MSR_IR) | (1 << MSR_DR))) {
> -/* If we disactivated any translation, flush TLBs */
> -tlb_flush(cs, 1);
> }
>
> #ifdef TARGET_PPC64
> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
> index 271fddf..23b8ded 100644
> --- a/target-ppc/helper_regs.h
> +++ b/target-ppc/helper_regs.h
> @@ -41,12 +41,15 @@ static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
>
> static inline void hreg_compute_mem_idx(CPUPPCState *env)
> {
> +int high;
> +
> /* Precompute MMU index */
> if (msr_pr == 0 && msr_hv != 0) {
> -env->mmu_idx = 2;
> +high = 2;
> } else {
> -env->mmu_idx = 1 - msr_pr;
> +high = 1 - msr_pr;
> }
> +env->mmu_idx = (high << 2) | (msr_ir << 1) | msr_dr;
> }
>
> static inline void hreg_compute_hflags(CPUPPCState *env)
> @@ -56,7 +59,7 @@ static inline void hreg_compute_hflags(CPUPPCState *env)
> /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
> hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
> (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
> -(1 << MSR_LE) | (1 << MSR_VSX);
> +(1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
> hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> hreg_compute_mem_idx(env);
> env->hflags = env->msr & hflags_mask;
> @@ -82,8 +85,6 @@ static inline int hreg_store_msr(CPUPPCState *env,
> target_ulong value,
> }
> if (((value >> MSR_IR) & 1) != msr_ir ||
> ((value >> MSR_DR) & 1) != msr_dr) {
> -/* Flush all tlb when changing translation mode */
> -tlb_flush(cs, 1);
> excp = POWERPC_EXCP_NONE;
> cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> }
>
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On Tue, Sep 16, 2014 at 06:23:29PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 18:20, Peter Maydell ha scritto: > >> > - bumping the minimum required version from 2.12 to 2.16. I suggest > >> > bumping to the currently required version for Windows, which is 2.20 > >> > (released March 2009). > > The commit message for a52d28afb suggests that this bump would be > > dropping support for RHEL5. That doesn't seem like a great idea > > to me, especially if it's only for the benefit of a test program. > > Though you would just drop support for "make check" on RHEL5. Even > "make -k check" would roughly work. Probably not too hard to drop the specific test, right? > > On the other hand your patch doesn't actually bump the required > > version, so I'm a bit confused. > > Yeah, this is just an RFC. > > Paolo Message in the glib header hints at some reliability problems with g_test_fork. Any idea what these might be?
Re: [Qemu-devel] [PATCH] kvmclock: clarify usage of cpu_clean_all_dirty
On Tue, Sep 16, 2014 at 05:42:16PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 17:14, Marcelo Tosatti ha scritto: > > + /* > > +* Make sure that CPU state is synchronized from KVM > > +* once every VM state change callback has finished. > > Which other callback could affect the in-kernel state, Do you want me to find out exactly which callback is changing the in-kernel interrupt state? To be honest: i don't know. > and should that call cpu_clean_all_dirty instead? Does not make sense: any callback which modifies any in-kernel register state should clean dirty state because its possibly set.
Re: [Qemu-devel] [PATCH] kvmclock: clarify usage of cpu_clean_all_dirty
On Tue, Sep 16, 2014 at 01:48:24PM -0300, Marcelo Tosatti wrote: > On Tue, Sep 16, 2014 at 06:22:15PM +0200, Paolo Bonzini wrote: > > Il 16/09/2014 18:07, Marcelo Tosatti ha scritto: > > >> > The cpu_synchronize_all_states() call in kvmclock_vm_state_change() is > > >> > needed to make env->tsc up to date with the value on the source, right? > > > Its there to make sure the pair > > > > > > env->tsc, s->clock = data.clock > > > > > > are relative to point close in time. > > > > Ok. But why are they not close in time? > > Scenario 1 > > A. s->clock = get_clock() > B. env->tsc = rdtsc() > > Scenario 2 > > A. s->clock = get_clock() > C. VM callbacks, bdrv_flush, ... > B. env->tsc = rdtsc() > > They are not "close in time" because of C. > > > Could we have the opposite situation where env->tsc is loaded a long > > time _after_ s->clock, and something breaks? > > This particular read avoids an overflow. See > > +assert(time.tsc_timestamp <= migration_tsc); > > > About your question, perhaps, would have to make up an > env->tsc, s->clock pair which breaks the code or a guest > application. > > > Also, there is no reason to do kvmclock_current_nsec() during normal > > execution of the VM. Is the s->clock sent by the source ever good > > across migration, and could the source send kvmclock_current_nsec() > > instead of whatever KVM_GET_CLOCK returns? > > Yes it could. What difference does it make? > > > I don't understand this code very well, but it seems to me that the > > migration handling and VM state change handler are mixed up... > > I don't see what the problem is. I am sure you can understand the code. If you have a suggestion to improve the code, i am all ears. But IMHO, its not terrible (migration and VM state change handling are mixed in other drivers as well).
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On Tue, Sep 16, 2014 at 09:45:03AM -0700, Peter Maydell wrote: > On 16 September 2014 09:33, Paolo Bonzini wrote: > > Il 16/09/2014 18:28, Peter Maydell ha scritto: > >>> > Though you would just drop support for "make check" on RHEL5. Even > >>> > "make -k check" would roughly work. > >> Can't we just put in the makefile and configure magic to skip > >> the test if the glib version is too old for it? > > > > Yes, that's what Michael did (minus the magic to use the older > > fork-based code). But is it really worthwhile to support RHEL5? We > > made glib mandatory in 2011 (around RHEL6.2), and that broke RHEL4. By > > the time the next QEMU release is ready RHEL7.1 should be out, give or > > take a month or so. > > I care because in the EDA tools space things move very slowly > and so RHEL5 is still fairly commonplace. Obviously at some > point we're going to end up dropping support, but "one test > case won't build" seems like a pretty trivial reason to drop > it to me. > > -- PMM Could you pls tell me whether my patch works, in your testing? My box which had such an old glib seems to be dead.
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
On Tue, Sep 16, 2014 at 06:27:51PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 18:26, Michael S. Tsirkin ha scritto: > > Right so types should be explicit. > > If an arbitrary string isn't allowed, this should be documented. > > It's not great as is: what's the format for macaddr? AA:BB:CC:DD:EE:FF? > > aa:bb:cc:dd:ee:ff? aabbccddeeff? 0xaabbccddeeff? > > But just saying "string" is going in the wrong direction imho. > > That's the purpose of documentation (docs/qdev-device-use.txt), That's not user documentation, that's developer documentation, isn't it? > and even > then is better done with examples. I don't think doing it in -device > foo,help (which I'm not even sure is particularly helpful. -device foo,help isn't helpful at all because it does not tell people what does each option do. But it really should be fixed. > "ip link help" doesn't document the format of a MAC address, either. Networking on linux is notoriously hard to configure. Hence the rise of wrapper tools such as network manager to hide all the details from users. > I'm sympathetic towards fixing the drive->str change, but I have no idea > how to do it. > > Paolo Change legacy_name to point to a detailed human-readable description of the type? E.g. "Ethernet 6-byte MAC Address, format: AA:BB:CC:DD:EE:FF"? We really really should add description to all properties, too. -- MST
Re: [Qemu-devel] [PATCH] kvmclock: clarify usage of cpu_clean_all_dirty
On Tue, Sep 16, 2014 at 06:22:15PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 18:07, Marcelo Tosatti ha scritto: > >> > The cpu_synchronize_all_states() call in kvmclock_vm_state_change() is > >> > needed to make env->tsc up to date with the value on the source, right? > > Its there to make sure the pair > > > > env->tsc, s->clock = data.clock > > > > are relative to point close in time. > > Ok. But why are they not close in time? Scenario 1 A. s->clock = get_clock() B. env->tsc = rdtsc() Scenario 2 A. s->clock = get_clock() C. VM callbacks, bdrv_flush, ... B. env->tsc = rdtsc() They are not "close in time" because of C. > Could we have the opposite situation where env->tsc is loaded a long > time _after_ s->clock, and something breaks? This particular read avoids an overflow. See +assert(time.tsc_timestamp <= migration_tsc); About your question, perhaps, would have to make up an env->tsc, s->clock pair which breaks the code or a guest application. > Also, there is no reason to do kvmclock_current_nsec() during normal > execution of the VM. Is the s->clock sent by the source ever good > across migration, and could the source send kvmclock_current_nsec() > instead of whatever KVM_GET_CLOCK returns? Yes it could. What difference does it make? > I don't understand this code very well, but it seems to me that the > migration handling and VM state change handler are mixed up... I don't see what the problem is. I am sure you can understand the code.
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On 16 September 2014 09:33, Paolo Bonzini wrote: > Il 16/09/2014 18:28, Peter Maydell ha scritto: >>> > Though you would just drop support for "make check" on RHEL5. Even >>> > "make -k check" would roughly work. >> Can't we just put in the makefile and configure magic to skip >> the test if the glib version is too old for it? > > Yes, that's what Michael did (minus the magic to use the older > fork-based code). But is it really worthwhile to support RHEL5? We > made glib mandatory in 2011 (around RHEL6.2), and that broke RHEL4. By > the time the next QEMU release is ready RHEL7.1 should be out, give or > take a month or so. I care because in the EDA tools space things move very slowly and so RHEL5 is still fairly commonplace. Obviously at some point we're going to end up dropping support, but "one test case won't build" seems like a pretty trivial reason to drop it to me. -- PMM
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On Tue, Sep 16, 2014 at 09:28:03AM -0700, Peter Maydell wrote: > On 16 September 2014 09:23, Paolo Bonzini wrote: > > Il 16/09/2014 18:20, Peter Maydell ha scritto: > >>> > - bumping the minimum required version from 2.12 to 2.16. I suggest > >>> > bumping to the currently required version for Windows, which is 2.20 > >>> > (released March 2009). > >> The commit message for a52d28afb suggests that this bump would be > >> dropping support for RHEL5. That doesn't seem like a great idea > >> to me, especially if it's only for the benefit of a test program. > > > > Though you would just drop support for "make check" on RHEL5. Even > > "make -k check" would roughly work. > > Can't we just put in the makefile and configure magic to skip > the test if the glib version is too old for it? > > -- PMM That's exactly what my patch did. See 20140916144324.ga7...@redhat.com You prefer that then?
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
Il 16/09/2014 18:28, Peter Maydell ha scritto: >> > Though you would just drop support for "make check" on RHEL5. Even >> > "make -k check" would roughly work. > Can't we just put in the makefile and configure magic to skip > the test if the glib version is too old for it? Yes, that's what Michael did (minus the magic to use the older fork-based code). But is it really worthwhile to support RHEL5? We made glib mandatory in 2011 (around RHEL6.2), and that broke RHEL4. By the time the next QEMU release is ready RHEL7.1 should be out, give or take a month or so. Paolo
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On 16 September 2014 09:23, Paolo Bonzini wrote: > Il 16/09/2014 18:20, Peter Maydell ha scritto: >>> > - bumping the minimum required version from 2.12 to 2.16. I suggest >>> > bumping to the currently required version for Windows, which is 2.20 >>> > (released March 2009). >> The commit message for a52d28afb suggests that this bump would be >> dropping support for RHEL5. That doesn't seem like a great idea >> to me, especially if it's only for the benefit of a test program. > > Though you would just drop support for "make check" on RHEL5. Even > "make -k check" would roughly work. Can't we just put in the makefile and configure magic to skip the test if the glib version is too old for it? -- PMM
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
On Tue, Sep 16, 2014 at 12:45:43PM +0200, Paolo Bonzini wrote: > Il 16/09/2014 12:37, Michael S. Tsirkin ha scritto: > > str really should be for free-form strings. > > It makes as much sense to call it as string > > Yes, that's why I said drive->str is a degradation. The question is, > how important is that degradation? > > > as it is to call an integer a string because you type > > a string of characters to specify it. > > I disagree. This is true for the command line, and for historical > reasons it is also true for device-add, but the newer QOM commands are > type-safe. You cannot pass an integer as a JSON string '123' to qom-set > or object-add, for example. > > Paolo Right so types should be explicit. If an arbitrary string isn't allowed, this should be documented. It's not great as is: what's the format for macaddr? AA:BB:CC:DD:EE:FF? aa:bb:cc:dd:ee:ff? aabbccddeeff? 0xaabbccddeeff? But just saying "string" is going in the wrong direction imho. -- MST
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Il 16/09/2014 18:26, Michael S. Tsirkin ha scritto: > Right so types should be explicit. > If an arbitrary string isn't allowed, this should be documented. > It's not great as is: what's the format for macaddr? AA:BB:CC:DD:EE:FF? > aa:bb:cc:dd:ee:ff? aabbccddeeff? 0xaabbccddeeff? > But just saying "string" is going in the wrong direction imho. That's the purpose of documentation (docs/qdev-device-use.txt), and even then is better done with examples. I don't think doing it in -device foo,help (which I'm not even sure is particularly helpful. "ip link help" doesn't document the format of a MAC address, either. I'm sympathetic towards fixing the drive->str change, but I have no idea how to do it. Paolo
Re: [Qemu-devel] [PATCH v5 2/2] dump: Don't return error code when return an Error object
zhanghailiang writes:
> Functions shouldn't return an error code and an Error object at the same time.
> Turn all these functions that returning Error object to void.
> We also judge if a function success or fail by reference to the *errp.
>
> Signed-off-by: zhanghailiang
> ---
> dump.c | 244
> +
> 1 file changed, 94 insertions(+), 150 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index 07d2300..b2ed846 100644
> --- a/dump.c
> +++ b/dump.c
[...]
> /* write the memroy to vmcore. 1 page per I/O. */
> -static int write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t
> start,
> +static void write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t
> start,
> int64_t size, Error **errp)
> {
> int64_t i;
> -int ret;
>
> for (i = 0; i < size / TARGET_PAGE_SIZE; i++) {
> -ret = write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
> +write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
> TARGET_PAGE_SIZE, errp);
Please fix up indentation of arguments.
> -if (ret < 0) {
> -return ret;
> +if (*errp) {
> +return;
This breaks when a caller choses to ignore errors by passing a null
argument for errp.
For static functions that may be okay. But in general, we support null
arguments by coding like this:
Error *local_err = NULL;
int64_t i;
for (i = 0; i < size / TARGET_PAGE_SIZE; i++) {
write_data(s, block->host_addr + start + i * TARGET_PAGE_SIZE,
TARGET_PAGE_SIZE, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
}
I feel it's better to stick to this convention.
More of the same elsewhere.
> }
> }
[...]
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
Il 16/09/2014 18:20, Peter Maydell ha scritto: >> > - bumping the minimum required version from 2.12 to 2.16. I suggest >> > bumping to the currently required version for Windows, which is 2.20 >> > (released March 2009). > The commit message for a52d28afb suggests that this bump would be > dropping support for RHEL5. That doesn't seem like a great idea > to me, especially if it's only for the benefit of a test program. Though you would just drop support for "make check" on RHEL5. Even "make -k check" would roughly work. > On the other hand your patch doesn't actually bump the required > version, so I'm a bit confused. Yeah, this is just an RFC. Paolo
Re: [Qemu-devel] [PATCH] kvmclock: clarify usage of cpu_clean_all_dirty
Il 16/09/2014 18:07, Marcelo Tosatti ha scritto: >> > The cpu_synchronize_all_states() call in kvmclock_vm_state_change() is >> > needed to make env->tsc up to date with the value on the source, right? > Its there to make sure the pair > > env->tsc, s->clock = data.clock > > are relative to point close in time. Ok. But why are they not close in time? Could we have the opposite situation where env->tsc is loaded a long time _after_ s->clock, and something breaks? Also, there is no reason to do kvmclock_current_nsec() during normal execution of the VM. Is the s->clock sent by the source ever good across migration, and could the source send kvmclock_current_nsec() instead of whatever KVM_GET_CLOCK returns? I don't understand this code very well, but it seems to me that the migration handling and VM state change handler are mixed up... Paolo >> > But if the synchronize_all_states+clean_all_dirty pair runs on the >> > source, why is the cpu_synchronize_all_states() call in >> > qemu_savevm_state_complete() not enough? It runs even later than >> > kvmclock_vm_state_change. > Because of the "pair of time values" explanation above.
Re: [Qemu-devel] [RFC PATCH] tests: use g_test_trap_fork for glib between 2.16 and 2.38
On 16 September 2014 08:43, Paolo Bonzini wrote: > Glib recently introduced a robust way to run tests in a subprocess, > which is used in test-qdev-global-props. However, we would like > to have the same tests run with older versions of glib, and the > older fork-based mechanisms works well enough. > > This still requires: > > - bumping the minimum required version from 2.12 to 2.16. I suggest > bumping to the currently required version for Windows, which is 2.20 > (released March 2009). The commit message for a52d28afb suggests that this bump would be dropping support for RHEL5. That doesn't seem like a great idea to me, especially if it's only for the benefit of a test program. On the other hand your patch doesn't actually bump the required version, so I'm a bit confused. -- PMM
Re: [Qemu-devel] [PATCH v2 0/2] usb: Don't use qerror_report
Gerd Hoffmann writes:
> Hi,
>
>> > Gonglei (2):
>> > redirect.c: Don't use qerror_report()
>> > dev-network: Don't use qerror_report_err()
>
>> Hi, Gerd
>> Would you like to apply this patch series at present? Thanks!
>
> Picked it up now. Originally dropped due to review comments from paolo,
> but switching all usb over to realize is a bigger project indeed.
Please don't, it degrades QMP errors. Example:
{ "execute": "device_add", "arguments": { "driver": "usb-redir" } }
Reply before:
{"error": {"class": "GenericError", "desc": "Parameter 'chardev' is
missing"}}
After:
{"error": {"class": "GenericError", "desc": "Device initialization
failed."}}
with "Parameter 'chardev' is missing" barfed to stderr.
