[Qemu-devel] [PATCH] fix multiboot loading if load_end_addr == 0
The previous multiboot load code did not treat the case where
load_end_addr was 0 specially. The multiboot specification says the
following:
* load_end_addr
Contains the physical address of the end of the data segment.
(load_end_addr - load_addr) specifies how much data to load. This
implies that the text and data segments must be consecutive in the
OS image; this is true for existing a.out executable formats. If
this field is zero, the boot loader assumes that the text and data
segments occupy the whole OS image file.
Signed-off-by: Scott Moser
diff --git a/hw/multiboot.c b/hw/multiboot.c
index b4484a3..b1e04c5 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -202,10 +202,16 @@ int load_multiboot(void *fw_cfg,
uint32_t mh_bss_end_addr = ldl_p(header+i+24);
mh_load_addr = ldl_p(header+i+16);
uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
-uint32_t mb_load_size = mh_load_end_addr - mh_load_addr;
-
+uint32_t mb_load_size = 0;
mh_entry_addr = ldl_p(header+i+28);
-mb_kernel_size = mh_bss_end_addr - mh_load_addr;
+
+if (mh_load_end_addr) {
+mb_kernel_size = mh_bss_end_addr - mh_load_addr;
+mb_load_size = mh_load_end_addr - mh_load_addr;
+} else {
+mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
+mb_load_size = mb_kernel_size;
+}
/* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
uint32_t mh_mode_type = ldl_p(header+i+32);
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 2012-03-26 19:33, Anthony Liguori wrote: > On 03/26/2012 07:20 AM, Jan Kiszka wrote: >> On 2012-03-26 04:06, Wanpeng Li wrote: >>> From: Anthony Liguori >>> >>> >>> This series aggressively refactors the PC machine initialization to be more >>> modelled and less ad-hoc. The highlights of this series are: >>> >>> 1) Things like -m and -bios-name are now device model properties >>> >>> 2) The i440fx and piix3 are now modelled in a thorough fashion >>> >>> 3) Most of the chipset features of the piix3 are modelled through >>> composition >>> >>> 4) i440fx_init is trivialized to creating devices and setting properties >>> >>> 5) convert MemoryRegion to QOM >>> >>> 6) convert PCI host bridge to QOM >>> >>> The point (4) is the most important one. As we refactor in this fashion, >>> we should quickly get to the point where machine->init disappears >>> completely in >>> favor of just creating a handful of devices. >>> >>> The two stage initialization of QOM is important here. instance_init() is >>> when >>> composed devices are created which means that after you've created a >>> device, all >>> of its children are visible in the device model. This lets you set >>> properties >>> of the parent and its children. >>> >>> realize() (which is still called DeviceState::init today) will be called >>> right >>> before the guest starts up for the first time. >> >> While I see the value of the overall direction, I still disagree on >> making internal data structures of HPET, RTC and 8254 publicly >> available. That's a wrong step back. I'm sure there are smarter >> solutions, alse as there were some proposals back then in the original >> thread. > > I'm not fully decided myself. A couple things are clear to me though: > > 1) We must expose type proper types in header files. We need there to be a > globally accessible RTCState type and functions that operate on it. I'm not sure what "proper type" means in this context, but I'm quite sure that there should be no need for poking into the internal of the class outside of mc146818rtc.c. We even abstracted the specifics of the RTC away when it is embedded into a super-IO and interacts with an HPET. If QOM requires such poking, then that requirement should be reassessed. > > 2) We can simplify memory management by knowing the size of the type in the > header files too. Is this more than a malloc-free pair? > > Since this is an easily refactorable thing to look at later, I think we > should > start with extracting the types. My worry is that those three refactorings set bad examples for others. So I'd like to avoid such back and forth if possible. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one
Quoting Brian Jackson (i...@theiggy.com):
> On Mon, 26 Mar 2012 10:13:40 -0500, Serge E. Hallyn
> wrote:
>
> >Currently, if the user doesn't pass a uuid, the system uuid is set to
> >all zeros. This patch generates a random one instead.
> >
> >Is there a reason to prefer all zeros? If not, can a patch like this
> >one be applied?
> >
> >Signed-off-by: Serge Hallyn
> >---
> > vl.c | 11 +++
> > 1 files changed, 11 insertions(+), 0 deletions(-)
> >
> >diff --git a/vl.c b/vl.c
> >index 112b0e0..2b53b62 100644
> >--- a/vl.c
> >+++ b/vl.c
> >@@ -247,7 +247,9 @@ int nb_numa_nodes;
> > uint64_t node_mem[MAX_NODES];
> > uint64_t node_cpumask[MAX_NODES];
> >+#include
>
>
>
> This adds a hard dep on libuuid (prior to this it's optional and
> only impacts the vdi block driver).
Yup. If that's a problem we can re-implement our own, if the
change in default behavior is acceptable.
> > uint8_t qemu_uuid[16];
> >+bool uuid_set = false;
> >static QEMUBootSetHandler *boot_set_handler;
> > static void *boot_set_opaque;
> >@@ -3030,6 +3032,7 @@ int main(int argc, char **argv, char **envp)
> > " Wrong format.\n");
> > exit(1);
> > }
> >+uuid_set = true;
> > break;
> > case QEMU_OPTION_option_rom:
> > if (nb_option_roms >= MAX_OPTION_ROMS) {
> >@@ -3200,6 +3203,14 @@ int main(int argc, char **argv, char **envp)
> > exit(0);
> > }
> >+if (!uuid_set) {
> >+uuid_t uuid;
> >+uuid_generate(uuid);
> >+for (i = 0; i < 16; i++) {
> >+qemu_uuid[i] = uuid[i];
> >+}
> >+}
> >+
> > /* Open the logfile at this point, if necessary. We can't
> >open the logfile
> > * when encountering either of the logging options (-d or -D)
> >because the
> > * other one may be encountered later on the command line,
> >changing the
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 03/26/2012 02:30 PM, Jan Kiszka wrote: On 2012-03-26 19:33, Anthony Liguori wrote: On 03/26/2012 07:20 AM, Jan Kiszka wrote: On 2012-03-26 04:06, Wanpeng Li wrote: From: Anthony Liguori This series aggressively refactors the PC machine initialization to be more modelled and less ad-hoc. The highlights of this series are: 1) Things like -m and -bios-name are now device model properties 2) The i440fx and piix3 are now modelled in a thorough fashion 3) Most of the chipset features of the piix3 are modelled through composition 4) i440fx_init is trivialized to creating devices and setting properties 5) convert MemoryRegion to QOM 6) convert PCI host bridge to QOM The point (4) is the most important one. As we refactor in this fashion, we should quickly get to the point where machine->init disappears completely in favor of just creating a handful of devices. The two stage initialization of QOM is important here. instance_init() is when composed devices are created which means that after you've created a device, all of its children are visible in the device model. This lets you set properties of the parent and its children. realize() (which is still called DeviceState::init today) will be called right before the guest starts up for the first time. While I see the value of the overall direction, I still disagree on making internal data structures of HPET, RTC and 8254 publicly available. That's a wrong step back. I'm sure there are smarter solutions, alse as there were some proposals back then in the original thread. I'm not fully decided myself. A couple things are clear to me though: 1) We must expose type proper types in header files. We need there to be a globally accessible RTCState type and functions that operate on it. I'm not sure what "proper type" means in this context, but I'm quite sure that there should be no need for poking into the internal of the class outside of mc146818rtc.c. It needs to be at least a forward reference. So we can avoid stuff like: int apic_accept_pic_intr(DeviceState *s); It should be: int apic_accept_pic_intr(APICState *s); So we can make use of the lovely type checking provided by the compiler to us. We even abstracted the specifics of the RTC away when it is embedded into a super-IO and interacts with an HPET. If QOM requires such poking, then that requirement should be reassessed. There are a couple of ways to make types private while still having forward declarations. None of them are straight forward. That's why I suggest we save this for another day. 2) We can simplify memory management by knowing the size of the type in the header files too. Is this more than a malloc-free pair? Since this is an easily refactorable thing to look at later, I think we should start with extracting the types. My worry is that those three refactorings set bad examples for others. So I'd like to avoid such back and forth if possible. I'm not really worried about it. It's so easier to refactor this later. Why rush it now? Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 2012-03-26 21:35, Anthony Liguori wrote: > On 03/26/2012 02:30 PM, Jan Kiszka wrote: >> On 2012-03-26 19:33, Anthony Liguori wrote: >>> On 03/26/2012 07:20 AM, Jan Kiszka wrote: On 2012-03-26 04:06, Wanpeng Li wrote: > From: Anthony Liguori > > > This series aggressively refactors the PC machine initialization to > be more > modelled and less ad-hoc. The highlights of this series are: > >1) Things like -m and -bios-name are now device model properties > >2) The i440fx and piix3 are now modelled in a thorough fashion > >3) Most of the chipset features of the piix3 are modelled > through composition > >4) i440fx_init is trivialized to creating devices and setting > properties > >5) convert MemoryRegion to QOM > >6) convert PCI host bridge to QOM > > The point (4) is the most important one. As we refactor in this > fashion, > we should quickly get to the point where machine->init disappears > completely in > favor of just creating a handful of devices. > > The two stage initialization of QOM is important here. > instance_init() is when > composed devices are created which means that after you've created > a device, all > of its children are visible in the device model. This lets you set > properties > of the parent and its children. > > realize() (which is still called DeviceState::init today) will be > called right > before the guest starts up for the first time. While I see the value of the overall direction, I still disagree on making internal data structures of HPET, RTC and 8254 publicly available. That's a wrong step back. I'm sure there are smarter solutions, alse as there were some proposals back then in the original thread. >>> >>> I'm not fully decided myself. A couple things are clear to me though: >>> >>> 1) We must expose type proper types in header files. We need there >>> to be a >>> globally accessible RTCState type and functions that operate on it. >> >> I'm not sure what "proper type" means in this context, but I'm quite >> sure that there should be no need for poking into the internal of the >> class outside of mc146818rtc.c. > > It needs to be at least a forward reference. So we can avoid stuff like: > > int apic_accept_pic_intr(DeviceState *s); > > It should be: > > int apic_accept_pic_intr(APICState *s); > > So we can make use of the lovely type checking provided by the compiler > to us. I do not disagree. A pointer is harmless. > >> We even abstracted the specifics of the >> RTC away when it is embedded into a super-IO and interacts with an HPET. >> If QOM requires such poking, then that requirement should be reassessed. > > There are a couple of ways to make types private while still having > forward declarations. None of them are straight forward. That's why I > suggest we save this for another day. > >>> >>> 2) We can simplify memory management by knowing the size of the type >>> in the >>> header files too. >> >> Is this more than a malloc-free pair? >> >>> >>> Since this is an easily refactorable thing to look at later, I think >>> we should >>> start with extracting the types. >> >> My worry is that those three refactorings set bad examples for others. >> So I'd like to avoid such back and forth if possible. > > I'm not really worried about it. It's so easier to refactor this > later. Why rush it now? You rush changing the current layout, not me. :) Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 03/26/2012 02:37 PM, Jan Kiszka wrote: On 2012-03-26 21:35, Anthony Liguori wrote: Since this is an easily refactorable thing to look at later, I think we should start with extracting the types. My worry is that those three refactorings set bad examples for others. So I'd like to avoid such back and forth if possible. I'm not really worried about it. It's so easier to refactor this later. Why rush it now? You rush changing the current layout, not me. :) No, I'm trying to do incremental changes without boiling the ocean in the process. I think we all are in violent agreement about where we want to end up (as opaque types as possible). I don't want to hold back additional refactoring on doing this right (and it's not just a matter of malloc/free). Regards, Anthony Liguori Jan
Re: [Qemu-devel] [RFC PATCH 06/17] block: use bdrv_{co, aio}_discard for write_zeroes operations
On Sat, 2012-03-24 at 16:27 +0100, Christoph Hellwig wrote: > > has_discard = !fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | > > FALLOC_FL_KEEP_SIZE, > > There is no point in using FALLOC_FL_KEEP_SIZE together with > FALLOC_FL_PUNCH_HOLE. It's *required*. From the man page [0], "The FALLOC_FL_PUNCH_HOLE flag must be ORed with FALLOC_FL_KEEP_SIZE in mode;" [0] http://man7.org/linux/man-pages/man2/fallocate.2.html -- Richard signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] [RFC PATCH 06/17] block: use bdrv_{co, aio}_discard for write_zeroes operations
On Sat, 2012-03-24 at 16:30 +0100, Christoph Hellwig wrote: > On Wed, Mar 14, 2012 at 01:14:18PM +0100, Paolo Bonzini wrote: > > > > Note that the discard granularity is only a hint, so it's really more a > > maximum suggested value than a granularity. Outside of a cluster > > boundary the format would still have to write zeros manually. > > > > Also, Linux for example will only round the number of sectors down to > > the granularity, not the start sector. > > Which really is more of a bug than a feature. The current discard > implementation in Linux is still very poor. The disk protocols do not require the granularity to be respected. It is *only a hint*. Therefore, QEMU has to handle non-aligned discards. It doesn't really matter what we wish was the case; this is the reality. -- Richard signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one
On 03/26/2012 10:13 AM, Serge E. Hallyn wrote:
Currently, if the user doesn't pass a uuid, the system uuid is set to
all zeros. This patch generates a random one instead.
Is there a reason to prefer all zeros? If not, can a patch like this
one be applied?
Signed-off-by: Serge Hallyn
The other hypervisors don't have a concept of a transient guest like QEMU does.
There is no state preserved between invocations of QEMU.
Setting a random UUID doesn't seem like the right answer to me as it would
potentially break Windows VMs.
Perhaps if the DMI UUID isn't set, you could look at the root filesystem's UUID?
Not all platforms have a notion of platform UUID so as Ubuntu supports more
architectures, this problem would have to be dealt with eventually.
Regards,
Anthony Liguori
---
vl.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/vl.c b/vl.c
index 112b0e0..2b53b62 100644
--- a/vl.c
+++ b/vl.c
@@ -247,7 +247,9 @@ int nb_numa_nodes;
uint64_t node_mem[MAX_NODES];
uint64_t node_cpumask[MAX_NODES];
+#include
uint8_t qemu_uuid[16];
+bool uuid_set = false;
static QEMUBootSetHandler *boot_set_handler;
static void *boot_set_opaque;
@@ -3030,6 +3032,7 @@ int main(int argc, char **argv, char **envp)
" Wrong format.\n");
exit(1);
}
+uuid_set = true;
break;
case QEMU_OPTION_option_rom:
if (nb_option_roms>= MAX_OPTION_ROMS) {
@@ -3200,6 +3203,14 @@ int main(int argc, char **argv, char **envp)
exit(0);
}
+if (!uuid_set) {
+uuid_t uuid;
+uuid_generate(uuid);
+for (i = 0; i< 16; i++) {
+qemu_uuid[i] = uuid[i];
+}
+}
+
/* Open the logfile at this point, if necessary. We can't open the logfile
* when encountering either of the logging options (-d or -D) because the
* other one may be encountered later on the command line, changing the
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 2012-03-26 21:39, Anthony Liguori wrote: > On 03/26/2012 02:37 PM, Jan Kiszka wrote: >> On 2012-03-26 21:35, Anthony Liguori wrote: > Since this is an easily refactorable thing to look at later, I think > we should > start with extracting the types. My worry is that those three refactorings set bad examples for others. So I'd like to avoid such back and forth if possible. >>> >>> I'm not really worried about it. It's so easier to refactor this >>> later. Why rush it now? >> >> You rush changing the current layout, not me. :) > > No, I'm trying to do incremental changes without boiling the ocean in > the process. > > I think we all are in violent agreement about where we want to end up > (as opaque types as possible). I don't want to hold back additional > refactoring on doing this right (and it's not just a matter of > malloc/free). Either I'm missing it in the code shuffling, or it's not part of this series: Can you point out where more that a forward reference and malloc/free is needed? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 03/26/2012 02:44 PM, Jan Kiszka wrote: On 2012-03-26 21:39, Anthony Liguori wrote: On 03/26/2012 02:37 PM, Jan Kiszka wrote: On 2012-03-26 21:35, Anthony Liguori wrote: Since this is an easily refactorable thing to look at later, I think we should start with extracting the types. My worry is that those three refactorings set bad examples for others. So I'd like to avoid such back and forth if possible. I'm not really worried about it. It's so easier to refactor this later. Why rush it now? You rush changing the current layout, not me. :) No, I'm trying to do incremental changes without boiling the ocean in the process. I think we all are in violent agreement about where we want to end up (as opaque types as possible). I don't want to hold back additional refactoring on doing this right (and it's not just a matter of malloc/free). Either I'm missing it in the code shuffling, or it's not part of this series: Can you point out where more that a forward reference and malloc/free is needed? QOM is built around the concept that the type size is know (as is GObject). type_initialize() assumes that the pointer passed is an adequate size. You would either need to move to a model where the memory was completely owned by QOM (which would mean folding type_new into type_initialize) or have a way to query instance size for a given type. This would also mean that reference counting should be revisited although with how dereferencing a parent affects the child. It's not rocket science, but it's also something that needs to be done carefully. Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 03/26/2012 02:44 PM, Jan Kiszka wrote: On 2012-03-26 21:39, Anthony Liguori wrote: On 03/26/2012 02:37 PM, Jan Kiszka wrote: On 2012-03-26 21:35, Anthony Liguori wrote: Since this is an easily refactorable thing to look at later, I think we should start with extracting the types. My worry is that those three refactorings set bad examples for others. So I'd like to avoid such back and forth if possible. I'm not really worried about it. It's so easier to refactor this later. Why rush it now? You rush changing the current layout, not me. :) No, I'm trying to do incremental changes without boiling the ocean in the process. I think we all are in violent agreement about where we want to end up (as opaque types as possible). I don't want to hold back additional refactoring on doing this right (and it's not just a matter of malloc/free). Either I'm missing it in the code shuffling, or it's not part of this series: Can you point out where more that a forward reference and malloc/free is needed? Inheritance is the other nasty case. To inherit from a type, you need to have the type definition. This is a pretty common problem with Object systems and the typical solution is PIMPL[1]. So maybe that's the right thing to do here, but that would have a significant affect on the code. That's what I mean by rushing how to handle this. There are multiple possible solutions and they need to be considered. The problem is purely aesthetic too, so I don't see a rush to fix it. [1] http://c2.com/cgi/wiki?PimplIdiom Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH v2] test: remove qemu-ga reference
On 03/21/2012 10:10 AM, Michael Roth wrote: This was added by mistake a while back. Signed-off-by: Michael Roth Applied. Thanks. Regards, Anthony Liguori --- Makefile |1 + tests/Makefile |2 +- 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 1bc3cb0..cab7c74 100644 --- a/Makefile +++ b/Makefile @@ -173,6 +173,7 @@ qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(qapi-obj-y): $(GENERATED_HEADERS) qapi-dir := $(BUILD_DIR)/qapi-generated qemu-ga$(EXESUF): LIBS = $(LIBS_QGA) +qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir) gen-out-type = $(subst .,-,$(suffix $@)) diff --git a/tests/Makefile b/tests/Makefile index c78ade1..354fdbb 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -17,7 +17,7 @@ test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) test-qmp-input-visitor.o test-qmp-output-visitor.o \ test-string-input-visitor.o test-string-output-visitor.o \ - test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir) + test-qmp-commands.o: QEMU_CFLAGS += -I $(qapi-dir) $(qapi-dir)/test-qapi-types.c $(qapi-dir)/test-qapi-types.h :\ $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
Re: [Qemu-devel] [PATCH v3] qapi: add c_fun to escape function names
On 03/20/2012 08:54 AM, Federico Simoncelli wrote:
Signed-off-by: Federico Simoncelli
Applied. Thanks.
Regards,
Anthony Liguori
---
scripts/qapi-commands.py | 14 +++---
scripts/qapi-types.py|4 ++--
scripts/qapi-visit.py|4 ++--
scripts/qapi.py |5 -
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 3aabf61..30a24d2 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -42,7 +42,7 @@ def generate_command_decl(name, args, ret_type):
return mcgen('''
%(ret_type)s qmp_%(name)s(%(args)sError **errp);
''',
- ret_type=c_type(ret_type), name=c_var(name),
args=arglist).strip()
+ ret_type=c_type(ret_type), name=c_fun(name),
args=arglist).strip()
def gen_sync_call(name, args, ret_type, indent=0):
ret = ""
@@ -59,7 +59,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
%(retval)sqmp_%(name)s(%(args)serrp);
''',
-name=c_var(name), args=arglist, retval=retval).rstrip()
+name=c_fun(name), args=arglist, retval=retval).rstrip()
if ret_type:
ret += "\n" + mcgen(
if (!error_is_set(errp)) {
@@ -74,7 +74,7 @@ if (!error_is_set(errp)) {
def gen_marshal_output_call(name, ret_type):
if not ret_type:
return ""
-return "qmp_marshal_output_%s(retval, ret, errp);" % c_var(name)
+return "qmp_marshal_output_%s(retval, ret, errp);" % c_fun(name)
def gen_visitor_output_containers_decl(ret_type):
ret = ""
@@ -198,16 +198,16 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s
ret_in, QObject **ret_o
qapi_dealloc_visitor_cleanup(md);
}
''',
-c_ret_type=c_type(ret_type), c_name=c_var(name),
+c_ret_type=c_type(ret_type), c_name=c_fun(name),
visitor=type_visitor(ret_type))
return ret
def gen_marshal_input_decl(name, args, ret_type, middle_mode):
if middle_mode:
-return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict,
QObject **ret)' % c_var(name)
+return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict,
QObject **ret)' % c_fun(name)
else:
-return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret,
Error **errp)' % c_var(name)
+return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret,
Error **errp)' % c_fun(name)
@@ -298,7 +298,7 @@ def gen_registry(commands):
registry += mcgen('''
qmp_register_command("%(name)s", qmp_marshal_input_%(c_name)s);
''',
- name=cmd['command'], c_name=c_var(cmd['command']))
+ name=cmd['command'], c_name=c_fun(cmd['command']))
pop_indent()
ret = mcgen('''
static void qmp_init_marshal(void)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 727fb77..4a734f5 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -100,7 +100,7 @@ typedef enum %(name)s
%(abbrev)s_%(value)s = %(i)d,
''',
abbrev=de_camel_case(name).upper(),
- value=c_var(value).upper(),
+ value=c_fun(value).upper(),
i=i)
i += 1
@@ -126,7 +126,7 @@ struct %(name)s
%(c_type)s %(c_name)s;
''',
c_type=c_type(typeinfo[key]),
- c_name=c_var(key))
+ c_name=c_fun(key))
ret += mcgen('''
};
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 54117d4..78c947c 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -129,9 +129,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const
char *name, Error **
break;
''',
abbrev = de_camel_case(name).upper(),
-enum = de_camel_case(key).upper(),
+enum = c_fun(de_camel_case(key)).upper(),
c_type=members[key],
-c_name=c_var(key))
+c_name=c_fun(key))
ret += mcgen('''
default:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6e05469..e062336 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -131,7 +131,10 @@ def camel_case(name):
return new_name
def c_var(name):
-return '_'.join(name.split('-')).lstrip("*")
+return name.replace('-', '_').lstrip("*")
+
+def c_fun(name):
+return c_var(name).replace('.', '_')
def c_list_type(name):
return '%sList' % name
Re: [Qemu-devel] [PATCH v4] Man page: Add -global description
On 03/21/2012 07:46 AM, Miroslav Rezanina wrote:
There's only TODO information in qemu man page for -global option. This is a
basic description of this option with simple example.
Signed-off-by: Miroslav Rezanina
Applied. Thanks.
Regards,
Anthony Liguori
v4:
- break long line
v3:
- add use case description
- use prop instead of property
v2:
- Use better value in example
Patch:
--
diff --git a/qemu-options.hx b/qemu-options.hx
index daefce3..662f571 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -288,13 +288,21 @@ TODO
ETEXI
DEF("global", HAS_ARG, QEMU_OPTION_global,
-"-global driver.property=value\n"
+"-global driver.prop=value\n"
"set a global default for a driver property\n",
QEMU_ARCH_ALL)
STEXI
-@item -global
+@item -global @var{driver}.@var{prop}=@var{value}
@findex -global
-TODO
+Set default value of @var{driver}'s property @var{prop} to @var{value}, e.g.:
+
+@example
+qemu -global ide-drive.physical_block_size=4096 -drive
file=file,if=ide,index=0,media=disk
+@end example
+
+In particular, you can use this to set driver properties for devices which are
+created automatically by the machine model. To create a device which is not
+created automatically and set properties on it, use -@option{device}.
ETEXI
DEF("mtdblock", HAS_ARG, QEMU_OPTION_mtdblock,
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 2012-03-26 21:49, Anthony Liguori wrote: > On 03/26/2012 02:44 PM, Jan Kiszka wrote: >> On 2012-03-26 21:39, Anthony Liguori wrote: >>> On 03/26/2012 02:37 PM, Jan Kiszka wrote: On 2012-03-26 21:35, Anthony Liguori wrote: >>> Since this is an easily refactorable thing to look at later, I think >>> we should >>> start with extracting the types. >> >> My worry is that those three refactorings set bad examples for >> others. >> So I'd like to avoid such back and forth if possible. > > I'm not really worried about it. It's so easier to refactor this > later. Why rush it now? You rush changing the current layout, not me. :) >>> >>> No, I'm trying to do incremental changes without boiling the ocean in >>> the process. >>> >>> I think we all are in violent agreement about where we want to end up >>> (as opaque types as possible). I don't want to hold back additional >>> refactoring on doing this right (and it's not just a matter of >>> malloc/free). >> >> Either I'm missing it in the code shuffling, or it's not part of this >> series: Can you point out where more that a forward reference and >> malloc/free is needed? > > QOM is built around the concept that the type size is know (as is > GObject). type_initialize() assumes that the pointer passed is an > adequate size. > > You would either need to move to a model where the memory was completely > owned by QOM (which would mean folding type_new into type_initialize) or > have a way to query instance size for a given type. > > This would also mean that reference counting should be revisited > although with how dereferencing a parent affects the child. > > It's not rocket science, but it's also something that needs to be done > carefully. But all this is only a problem for these three here (PIT, RTC, HPET) as their objects shall be embedded into the super-IO. If you only embed an object pointer, it shouldn't be an issue anymore, no? Also inheritance is not a problem here as we do not derive from the three types in question. If there is a super/sub-class relation, those need to share an internal header, of course. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 03/26/2012 03:10 PM, Jan Kiszka wrote: On 2012-03-26 21:49, Anthony Liguori wrote: On 03/26/2012 02:44 PM, Jan Kiszka wrote: This would also mean that reference counting should be revisited although with how dereferencing a parent affects the child. It's not rocket science, but it's also something that needs to be done carefully. But all this is only a problem for these three here (PIT, RTC, HPET) as their objects shall be embedded into the super-IO. If you only embed an object pointer, it shouldn't be an issue anymore, no? The reference counting stuff obviously needs to be looked at even in this case. Also inheritance is not a problem here as we do not derive from the three types in question. If there is a super/sub-class relation, those need to share an internal header, of course. Yes, but then you have two headers for every type. Is that really a good thing? I think I prefer to just mark fields as /*< private >*/ and scold people if they violate that convention. Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH v3 1/5] qerror: add error codes for fopen failure
On Fri, Mar 23, 2012 at 11:21:25AM -0300, Luiz Capitulino wrote:
> On Sun, 18 Mar 2012 19:29:09 +0100
> Alon Levy wrote:
>
> > Added:
> >
> > QERR_EINTR
> > QERR_EACCES
> > QERR_EEXIST
> > QERR_OPEN_FILE_EMFILE
> > QERR_ENOSPC
> > QERR_EPERM
> > QERR_READ_ONLY
> > QERR_ENOTDIR
> > QERR_EFBIG
> >
> > Signed-off-by: Alon Levy
> > ---
> > qerror.c | 36
> > qerror.h | 27 +++
> > 2 files changed, 63 insertions(+)
> >
> > diff --git a/qerror.c b/qerror.c
> > index f55d435..4915939 100644
> > --- a/qerror.c
> > +++ b/qerror.c
> > @@ -213,6 +213,42 @@ static const QErrorStringTable qerror_table[] = {
> > .desc = "Could not open '%(filename)'",
> > },
> > {
> > +.error_fmt = QERR_EINTR,
> > +.desc = "Interrupted open of '%(filename)'",
> > +},
>
> There are two problems here. First, the QERR_INTR macro doesn't have the
> filename argument, so this will crash.
>
> The second problem is that, I've talked to Anthony and EINTR should not
> be returned to clients, it should be handled intead. So, please handle it
> during write().
That would require either blocking or using async monitor... and in
general I guess it makes the signature of qemu_fopen_err be:
void qemu_fopen_err(const char *path, const char *mode, int block,
Error **errp)
Sounds ok?
>
> > +{
> > +.error_fmt = QERR_EACCES,
> > +.desc = "Cannot access file'",
> > +},
>
> We already have QERR_PERMISSION_DENIED.
>
> > +{
> > +.error_fmt = QERR_EEXIST,
> > +.desc = "File already exists'",
> > +},
>
> Can you use the description provided by man 3 errno? This is valid for all
> errors you're adding.
>
> > +{
> > +.error_fmt = QERR_OPEN_FILE_EMFILE,
> > +.desc = "Max open files when opening file'",
> > +},
> > +{
> > +.error_fmt = QERR_ENOSPC,
> > +.desc = "No space left opening file'",
> > +},
> > +{
> > +.error_fmt = QERR_EPERM,
> > +.desc = "Permission denied (EPERM) opening file'",
> > +},
> > +{
> > +.error_fmt = QERR_READ_ONLY,
> > +.desc = "Read only filesystem opening file'",
> > +},
> > +{
> > +.error_fmt = QERR_ENOTDIR,
> > +.desc = "Directory related error opening file'",
> > +},
> > +{
> > +.error_fmt = QERR_EFBIG,
> > +.desc = "File too big opening'",
> > +},
> > +{
> > .error_fmt = QERR_PERMISSION_DENIED,
> > .desc = "Insufficient permission to perform this operation",
> > },
> > diff --git a/qerror.h b/qerror.h
> > index e26c635..ddc04e8 100644
> > --- a/qerror.h
> > +++ b/qerror.h
> > @@ -181,6 +181,33 @@ QError *qobject_to_qerror(const QObject *obj);
> > #define QERR_OPEN_FILE_FAILED \
> > "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
> >
> > +#define QERR_OPEN_FILE_EMFILE \
> > +"{ 'class': 'OpenFileEMFILE', 'data': {} }"
> > +
> > +#define QERR_EINTR \
> > +"{ 'class': 'EINTR', 'data': {} }"
> > +
> > +#define QERR_EACCES \
> > +"{ 'class': 'EACCES', 'data': {} }"
> > +
> > +#define QERR_EEXIST \
> > +"{ 'class': 'EEXIST', 'data': {} }"
>
> We use more descriptive names instead of using the errno name. Like
> AlreadyExists.
> Please fix it for errors you adding.
>
> > +
> > +#define QERR_ENOSPC \
> > +"{ 'class': 'ENOSPC', 'data': {} }"
> > +
> > +#define QERR_EPERM \
> > +"{ 'class': 'EPERM', 'data': {} }"
> > +
> > +#define QERR_READ_ONLY \
> > +"{ 'class': 'ReadOnly', 'data': {} }"
> > +
> > +#define QERR_ENOTDIR \
> > +"{ 'class': 'ENOTDIR', 'data': {} }"
> > +
> > +#define QERR_EFBIG \
> > +"{ 'class': 'EFBIG', 'data': {} }"
> > +
> > #define QERR_PERMISSION_DENIED \
> > "{ 'class': 'PermissionDenied', 'data': {} }"
> >
>
>
Re: [Qemu-devel] [PATCH] qemu-ga: fix bsd build, and re-org linux-specific implementations
On 25/03/12 3:40 PM, Michael Roth wrote:
Signed-off-by: Michael Roth
---
qga/commands-posix.c | 111 +
1 files changed, 66 insertions(+), 45 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7b2be2f..faf970d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -12,29 +12,30 @@
*/
#include
+#include
+#include
+#include "qga/guest-agent-core.h"
+#include "qga-qmp-commands.h"
+#include "qerror.h"
+#include "qemu-queue.h"
+#include "host-utils.h"
#if defined(__linux__)
#include
#include
-
-#if defined(__linux__)&& defined(FIFREEZE)
-#define CONFIG_FSFREEZE
-#endif
-#endif
-
-#include
-#include
#include
#include
#include
#include
#include
-#include "qga/guest-agent-core.h"
-#include "qga-qmp-commands.h"
-#include "qerror.h"
-#include "qemu-queue.h"
-#include "host-utils.h"
+#if defined(__linux__)&& defined(FIFREEZE)
+#define CONFIG_FSFREEZE
+#endif
+#endif
+
+#if defined(__linux__)
+/* TODO: use this in place of all post-fork() fclose(std*) callers */
static void reopen_fd_to_null(int fd)
{
int nullfd;
@@ -50,6 +51,7 @@ static void reopen_fd_to_null(int fd)
close(nullfd);
}
}
+#endif /* defined(__linux__) */
void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
{
@@ -309,7 +311,11 @@ static void guest_file_init(void)
QTAILQ_INIT(&guest_file_state.filehandles);
}
+/* linux-specific implementations. avoid this if at all possible. */
+#if defined(__linux__)
+
#if defined(CONFIG_FSFREEZE)
+
static void disable_logging(void)
{
ga_disable_logging(ga_state);
@@ -505,38 +511,7 @@ static void guest_fsfreeze_cleanup(void)
}
}
}
-#else
-/*
- * Return status of freeze/thaw
- */
-GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
-{
-error_set(err, QERR_UNSUPPORTED);
-
-return 0;
-}
-
-/*
- * Walk list of mounted file systems in the guest, and freeze the ones which
- * are real local file systems.
- */
-int64_t qmp_guest_fsfreeze_freeze(Error **err)
-{
-error_set(err, QERR_UNSUPPORTED);
-
-return 0;
-}
-
-/*
- * Walk list of frozen file systems in the guest, and thaw them.
- */
-int64_t qmp_guest_fsfreeze_thaw(Error **err)
-{
-error_set(err, QERR_UNSUPPORTED);
-
-return 0;
-}
-#endif
+#endif /* CONFIG_FSFREEZE */
#define LINUX_SYS_STATE_FILE "/sys/power/state"
#define SUSPEND_SUPPORTED 0
@@ -904,6 +879,52 @@ error:
return NULL;
}
+#else /* defined(__linux__) */
+
+GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
+{
+error_set(err, QERR_UNSUPPORTED);
+
+return 0;
+}
+
+int64_t qmp_guest_fsfreeze_freeze(Error **err)
+{
+error_set(err, QERR_UNSUPPORTED);
+
+return 0;
+}
+
+int64_t qmp_guest_fsfreeze_thaw(Error **err)
+{
+error_set(err, QERR_UNSUPPORTED);
+
+return 0;
+}
+
+void qmp_guest_suspend_disk(Error **err)
+{
+error_set(err, QERR_UNSUPPORTED);
+}
+
+void qmp_guest_suspend_ram(Error **err)
+{
+error_set(err, QERR_UNSUPPORTED);
+}
+
+void qmp_guest_suspend_hybrid(Error **err)
+{
+error_set(err, QERR_UNSUPPORTED);
+}
+
+GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
+{
+error_set(errp, QERR_UNSUPPORTED);
+return NULL;
+}
+
+#endif
+
/* register init/cleanup routines for stateful command groups */
void ga_command_state_init(GAState *s, GACommandState *cs)
{
This builds. Thank you.
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 2012-03-26 22:13, Anthony Liguori wrote: > On 03/26/2012 03:10 PM, Jan Kiszka wrote: >> On 2012-03-26 21:49, Anthony Liguori wrote: >>> On 03/26/2012 02:44 PM, Jan Kiszka wrote: >>> This would also mean that reference counting should be revisited >>> although with how dereferencing a parent affects the child. >>> >>> It's not rocket science, but it's also something that needs to be done >>> carefully. >> >> But all this is only a problem for these three here (PIT, RTC, HPET) as >> their objects shall be embedded into the super-IO. If you only embed an >> object pointer, it shouldn't be an issue anymore, no? > > The reference counting stuff obviously needs to be looked at even in > this case. A composite object is owned by its container. So it should go when the container leaves. > >> Also inheritance is not a problem here as we do not derive from the >> three types in question. If there is a super/sub-class relation, those >> need to share an internal header, of course. > > Yes, but then you have two headers for every type. Is that really a > good thing? It's cleaner and more explicit than tagging members with comments. And it's nothing we will have for each and every type as only a small subset is actually inheriting, the mass is finalizing. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM
On 03/26/2012 03:30 PM, Jan Kiszka wrote: On 2012-03-26 22:13, Anthony Liguori wrote: On 03/26/2012 03:10 PM, Jan Kiszka wrote: On 2012-03-26 21:49, Anthony Liguori wrote: On 03/26/2012 02:44 PM, Jan Kiszka wrote: This would also mean that reference counting should be revisited although with how dereferencing a parent affects the child. It's not rocket science, but it's also something that needs to be done carefully. But all this is only a problem for these three here (PIT, RTC, HPET) as their objects shall be embedded into the super-IO. If you only embed an object pointer, it shouldn't be an issue anymore, no? The reference counting stuff obviously needs to be looked at even in this case. A composite object is owned by its container. So it should go when the container leaves. It's more complicated than that because it's a tree, not a graph. So there may be additional references held beyond the parent. Also inheritance is not a problem here as we do not derive from the three types in question. If there is a super/sub-class relation, those need to share an internal header, of course. Yes, but then you have two headers for every type. Is that really a good thing? It's cleaner and more explicit than tagging members with comments. And it's nothing we will have for each and every type as only a small subset is actually inheriting, the mass is finalizing. I don't fundamentally disagree with anything you're saying here. I'm only pointing out that (1) it's not a trivial problem and (2) it's not an urgent problem. Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PULL 0/2] spice: 32bit support
On 03/22/2012 08:40 AM, Gerd Hoffmann wrote: Hi, This patch series fixes the remaining 32bit bugs in spice. With this series merged and the latest spice-server bits (from git, no release yet) spice works on 32bit hosts too. please pull, Gerd Pulled. Thanks. Regards, Anthony Liguori The following changes since commit 33cf629a3754b58a1e2dbbe01d91d97e712b7c06: Merge remote-tracking branch 'sstabellini/saverestore-8' into staging (2012-03-19 13:39:42 -0500) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu spice.v51 Alon Levy (1): ui/spice-display: use uintptr_t when casting qxl physical addresses Peter Maydell (1): ui/spice-display.c: Fix compilation warnings on 32 bit hosts ui/spice-display.c | 22 +++--- 1 files changed, 11 insertions(+), 11 deletions(-)
Re: [Qemu-devel] [PULL] qemu-ga build fixes
On 03/26/2012 01:28 PM, Michael Roth wrote: The following changes since commit cb1977d308f6e1d6bf398d42e6148187b82456c1: tcg-sparc: Add debug_frame support. (2012-03-24 19:57:58 +) are available in the git repository at: git://github.com/mdroth/qemu.git qga-pull-3-26-12 Pulled. Thanks. Regards, Anthony Liguori Michael Roth (1): qemu-ga: fix bsd build, and re-org linux-specific implementations qga/commands-posix.c | 111 + 1 files changed, 66 insertions(+), 45 deletions(-)
Re: [Qemu-devel] [PULL 0/5] Trivial patches for 20 to 26 March 2012
On 03/26/2012 07:07 AM, Stefan Hajnoczi wrote: The following changes since commit cb1977d308f6e1d6bf398d42e6148187b82456c1: tcg-sparc: Add debug_frame support. (2012-03-24 19:57:58 +) are available in the git repository at: git://github.com/stefanha/qemu.git trivial-patches for you to fetch changes up to 95b752bc32ccabe48430c0d0062b7c6947d864d0: trace-events: Fix broken build caused by wrong format specifier (2012-03-26 12:34:20 +0100) Pulled. Thanks. Regards, Anthony Liguori Eduardo Habkost (1): vl.c: fix '-cpu ?' segfault Michael Roth (3): test: remove qemu-ga reference qapi: remove print statements from test-qmp-commands test: add test-qmp-commands to make check Stefan Weil (1): trace-events: Fix broken build caused by wrong format specifier Makefile|1 + test-qmp-commands.c |3 --- tests/Makefile |3 ++- trace-events|2 +- vl.c|2 +- 5 files changed, 5 insertions(+), 6 deletions(-)
Re: [Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest
On 03/16/2012 03:54 AM, Jason Wang wrote: This an update of series that let guest and qemu to be co-operated to send gratuitous packets when needed such as after migration, loadvm and continuing. As it's hard for qemu to track the network configuration in guest such as bondings, vlans or ipv6. So current gratuitous may not work under those situations. Can you be more specific about the failure scenarios? Does this mean that migration cannot work today with guests using ipv6? I don't think just pushing this to the guest is an acceptable solution in the short term. Are there scenarios we cannot handle no matter what in the host? Does this mean that for emulated drivers, we're completely out of luck? Regards, Anthony Liguori
[Qemu-devel] [PATCH v2] block stream: close unused files and update ->backing_hd
Close the now unused images that were part of the previous backing file
chain and adjust ->backing_hd, backing_filename and backing_format
properly.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=801449
Signed-off-by: Marcelo Tosatti
diff --git a/block/stream.c b/block/stream.c
index d1b3986..5880288 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -76,6 +76,39 @@ static int coroutine_fn stream_populate(BlockDriverState *bs,
return bdrv_co_copy_on_readv(bs, sector_num, nb_sectors, &qiov);
}
+static void close_unused_images(BlockDriverState *top, BlockDriverState *base,
+const char *base_id)
+{
+BlockDriverState *intermediate;
+intermediate = top->backing_hd;
+
+while (intermediate) {
+BlockDriverState *unused;
+
+/* reached base */
+if (intermediate == base) {
+break;
+}
+
+unused = intermediate;
+intermediate = intermediate->backing_hd;
+unused->backing_hd = NULL;
+bdrv_delete(unused);
+}
+top->backing_hd = base;
+
+pstrcpy(top->backing_file, sizeof(top->backing_file), "");
+pstrcpy(top->backing_format, sizeof(top->backing_format), "");
+if (base_id) {
+pstrcpy(top->backing_file, sizeof(top->backing_file), base_id);
+if (base->drv) {
+pstrcpy(top->backing_format, sizeof(top->backing_format),
+base->drv->format_name);
+}
+}
+
+}
+
/*
* Given an image chain: [BASE] -> [INTER1] -> [INTER2] -> [TOP]
*
@@ -221,6 +254,7 @@ retry:
base_id = s->backing_file_id;
}
ret = bdrv_change_backing_file(bs, base_id, NULL);
+close_unused_images(bs, base, base_id);
}
qemu_vfree(buf);
Re: [Qemu-devel] [PATCH 05/11 v10] Add API to get memory mapping
From: Wen Congyang
Subject: Re: [PATCH 05/11 v10] Add API to get memory mapping
Date: Mon, 26 Mar 2012 10:44:40 +0800
> At 03/26/2012 10:31 AM, HATAYAMA Daisuke Wrote:
>> From: Wen Congyang
>> Subject: Re: [PATCH 05/11 v10] Add API to get memory mapping
>> Date: Mon, 26 Mar 2012 09:10:52 +0800
>>
>>> At 03/23/2012 08:02 PM, HATAYAMA Daisuke Wrote:
From: Wen Congyang
Subject: [PATCH 05/11 v10] Add API to get memory mapping
Date: Tue, 20 Mar 2012 11:51:18 +0800
> Add API to get all virtual address and physical address mapping.
> If the guest doesn't use paging, the virtual address is equal to the
> phyical
> address. The virtual address and physical address mapping is for gdb's
> user, and
> it does not include the memory that is not referenced by the page table.
> So if
> you want to use crash to anaylze the vmcore, please do not specify -p
> option.
> the reason why the -p option is not default explicitly: guest machine in a
> catastrophic state can have corrupted memory, which we cannot trust.
>
> Signed-off-by: Wen Congyang
> ---
> memory_mapping.c | 34 ++
> memory_mapping.h | 15 +++
> 2 files changed, 49 insertions(+), 0 deletions(-)
>
> diff --git a/memory_mapping.c b/memory_mapping.c
> index 718f271..b92e2f6 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -164,3 +164,37 @@ void memory_mapping_list_init(MemoryMappingList
> *list)
> list->last_mapping = NULL;
> QTAILQ_INIT(&list->head);
> }
> +
> +#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
> +int qemu_get_guest_memory_mapping(MemoryMappingList *list)
> +{
> +CPUArchState *env;
> +RAMBlock *block;
> +ram_addr_t offset, length;
> +int ret;
> +bool paging_mode;
> +
> +paging_mode = cpu_paging_enabled(first_cpu);
> +if (paging_mode) {
On SMP with (n)-CPUs, we can do this check at most (n)-times.
On Linux, user-mode tasks have differnet page tables. If refering to
one page table, we can get one user-mode task memory only. Considering
as much memory as possible, it's best to reference all CPUs with
paging enabled and walk all the page tables.
A problem is that linear addresses for user-mode tasks can inherently
conflicts. Different user-mode tasks can have the same linear
address. So, tools need to distinguish each PT_LOAD entry based on a
pair of linear address and physical address, not linear address
only. I don't know whether gdb does this.
>>>
>>> gdb only can process kernel space. Jan's gdb-python script may can process
>>> user-mode tasks, but we should get user-mode task's register from the kernel
>>> or note, and convest virtual address/linear address to physicall address.
>>>
>>
>> After I send this, I came up with the problem of page tabel coherency:
>> some page table has not updated yet so we see older ones. So if we use
>
> Tha page table is older? Do you mean the newest page table is in TLB and
> is not flushed to memory?
>
I say vmalloc() in most part. (to be honest I don't know other
possibility now) In stable state of kernel, page tables are allocated
when user processes are created (around dup_mm()?, IIRC), where part
for kernel space is copied from init_mm.pgd. They are updated at
runtime coherently from init_mm.pgd when page fault happens. I
expressed the page table that has not updated yet as old. For this
reason, paging can lead to different result for differnet CPU.
>> all the page tables referenced by all CPUs, we face inconsistency of
>> some of the page tables. Essentially, we cannot avoid the issue that
>> we see the page table older than the actual even if we use only one
>> page table, but if restricting the use of page table to just one, we
>> can at least avoid the inconsistency of multiple page tables. In other
>> words, we can do paging processing normally though the table might be
>> old.
>>
>> So, I think
>> - using page tables for all the CPUs at the same time is problematic.
>> - using only one page table of the exsiting CPUs is still safe.
>>
>> How about the code like this?
>>
>> cpu = find_cpu_paging_enabled(env);
>
> If there are more than two cpu's paging is enabled, which cpu should be
> choosed?
> We cannot say which one is better than another one.
>
I think so too. But now it sees only one CPU. Seeing all CPUs in order
can increase possibility to do paging, which must be better if users
want to do paging.
Thanks.
HATAYAMA, Daisuke
[Qemu-devel] [PATCH] Make live saving stages public and use their #defines
Make the different stages for live saving of state public and use
the #defines rather than numbers.
Signed-off-by: Stefan Berger
---
arch_init.c |7 ---
block-migration.c |8
savevm.c |7 ---
vmstate.h |8
4 files changed, 16 insertions(+), 14 deletions(-)
Index: qemu/savevm.c
===
--- qemu.orig/savevm.c
+++ qemu/savevm.c
@@ -1541,13 +1541,6 @@ static void vmstate_save(QEMUFile *f, Sa
#define QEMU_VM_FILE_VERSION_COMPAT 0x0002
#define QEMU_VM_FILE_VERSION 0x0003
-#define QEMU_VM_EOF 0x00
-#define QEMU_VM_SECTION_START0x01
-#define QEMU_VM_SECTION_PART 0x02
-#define QEMU_VM_SECTION_END 0x03
-#define QEMU_VM_SECTION_FULL 0x04
-#define QEMU_VM_SUBSECTION 0x05
-
bool qemu_savevm_state_blocked(Error **errp)
{
SaveStateEntry *se;
Index: qemu/vmstate.h
===
--- qemu.orig/vmstate.h
+++ qemu/vmstate.h
@@ -31,6 +31,14 @@ typedef void SaveStateHandler(QEMUFile *
typedef int SaveLiveStateHandler(QEMUFile *f, int stage, void *opaque);
typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
+/* stages of the live state handler */
+#define QEMU_VM_EOF 0x00
+#define QEMU_VM_SECTION_START0x01
+#define QEMU_VM_SECTION_PART 0x02
+#define QEMU_VM_SECTION_END 0x03
+#define QEMU_VM_SECTION_FULL 0x04
+#define QEMU_VM_SUBSECTION 0x05
+
int register_savevm(DeviceState *dev,
const char *idstr,
int instance_id,
Index: qemu/arch_init.c
===
--- qemu.orig/arch_init.c
+++ qemu/arch_init.c
@@ -275,7 +275,7 @@ int ram_save_live(QEMUFile *f, int stage
memory_global_sync_dirty_bitmap(get_system_memory());
-if (stage == 1) {
+if (stage == QEMU_VM_SECTION_START) {
RAMBlock *block;
bytes_transferred = 0;
last_block = NULL;
@@ -330,7 +330,7 @@ int ram_save_live(QEMUFile *f, int stage
}
/* try transferring iterative blocks of memory */
-if (stage == 3) {
+if (stage == QEMU_VM_SECTION_END) {
int bytes_sent;
/* flush all remaining blocks regardless of rate limiting */
@@ -344,7 +344,8 @@ int ram_save_live(QEMUFile *f, int stage
expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
-return (stage == 2) && (expected_time <= migrate_max_downtime());
+return (stage == QEMU_VM_SECTION_PART) &&
+ (expected_time <= migrate_max_downtime());
}
static inline void *host_from_stream_offset(QEMUFile *f,
Index: qemu/block-migration.c
===
--- qemu.orig/block-migration.c
+++ qemu/block-migration.c
@@ -554,7 +554,7 @@ static int block_save_live(QEMUFile *f,
return 1;
}
-if (stage == 1) {
+if (stage == QEMU_VM_SECTION_START) {
init_blk_migration(f);
/* start track dirty blocks */
@@ -571,7 +571,7 @@ static int block_save_live(QEMUFile *f,
blk_mig_reset_dirty_cursor();
-if (stage == 2) {
+if (stage == QEMU_VM_SECTION_PART) {
/* control the rate of transfer */
while ((block_mig_state.submitted +
block_mig_state.read_done) * BLOCK_SIZE <
@@ -599,7 +599,7 @@ static int block_save_live(QEMUFile *f,
}
}
-if (stage == 3) {
+if (stage == QEMU_VM_SECTION_END) {
/* we know for sure that save bulk is completed and
all async read completed */
assert(block_mig_state.submitted == 0);
@@ -620,7 +620,7 @@ static int block_save_live(QEMUFile *f,
qemu_put_be64(f, BLK_MIG_FLAG_EOS);
-return ((stage == 2) && is_stage2_completed());
+return ((stage == QEMU_VM_SECTION_PART) && is_stage2_completed());
}
static int block_load(QEMUFile *f, void *opaque, int version_id)
Re: [Qemu-devel] [PATCH 05/11 v10] Add API to get memory mapping
At 03/27/2012 09:01 AM, HATAYAMA Daisuke Wrote:
> From: Wen Congyang
> Subject: Re: [PATCH 05/11 v10] Add API to get memory mapping
> Date: Mon, 26 Mar 2012 10:44:40 +0800
>
>> At 03/26/2012 10:31 AM, HATAYAMA Daisuke Wrote:
>>> From: Wen Congyang
>>> Subject: Re: [PATCH 05/11 v10] Add API to get memory mapping
>>> Date: Mon, 26 Mar 2012 09:10:52 +0800
>>>
At 03/23/2012 08:02 PM, HATAYAMA Daisuke Wrote:
> From: Wen Congyang
> Subject: [PATCH 05/11 v10] Add API to get memory mapping
> Date: Tue, 20 Mar 2012 11:51:18 +0800
>
>> Add API to get all virtual address and physical address mapping.
>> If the guest doesn't use paging, the virtual address is equal to the
>> phyical
>> address. The virtual address and physical address mapping is for gdb's
>> user, and
>> it does not include the memory that is not referenced by the page table.
>> So if
>> you want to use crash to anaylze the vmcore, please do not specify -p
>> option.
>> the reason why the -p option is not default explicitly: guest machine in
>> a
>> catastrophic state can have corrupted memory, which we cannot trust.
>>
>> Signed-off-by: Wen Congyang
>> ---
>> memory_mapping.c | 34 ++
>> memory_mapping.h | 15 +++
>> 2 files changed, 49 insertions(+), 0 deletions(-)
>>
>> diff --git a/memory_mapping.c b/memory_mapping.c
>> index 718f271..b92e2f6 100644
>> --- a/memory_mapping.c
>> +++ b/memory_mapping.c
>> @@ -164,3 +164,37 @@ void memory_mapping_list_init(MemoryMappingList
>> *list)
>> list->last_mapping = NULL;
>> QTAILQ_INIT(&list->head);
>> }
>> +
>> +#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
>> +int qemu_get_guest_memory_mapping(MemoryMappingList *list)
>> +{
>> +CPUArchState *env;
>> +RAMBlock *block;
>> +ram_addr_t offset, length;
>> +int ret;
>> +bool paging_mode;
>> +
>> +paging_mode = cpu_paging_enabled(first_cpu);
>> +if (paging_mode) {
>
> On SMP with (n)-CPUs, we can do this check at most (n)-times.
>
> On Linux, user-mode tasks have differnet page tables. If refering to
> one page table, we can get one user-mode task memory only. Considering
> as much memory as possible, it's best to reference all CPUs with
> paging enabled and walk all the page tables.
>
> A problem is that linear addresses for user-mode tasks can inherently
> conflicts. Different user-mode tasks can have the same linear
> address. So, tools need to distinguish each PT_LOAD entry based on a
> pair of linear address and physical address, not linear address
> only. I don't know whether gdb does this.
gdb only can process kernel space. Jan's gdb-python script may can process
user-mode tasks, but we should get user-mode task's register from the
kernel
or note, and convest virtual address/linear address to physicall address.
>>>
>>> After I send this, I came up with the problem of page tabel coherency:
>>> some page table has not updated yet so we see older ones. So if we use
>>
>> Tha page table is older? Do you mean the newest page table is in TLB and
>> is not flushed to memory?
>>
>
> I say vmalloc() in most part. (to be honest I don't know other
> possibility now) In stable state of kernel, page tables are allocated
> when user processes are created (around dup_mm()?, IIRC), where part
> for kernel space is copied from init_mm.pgd. They are updated at
> runtime coherently from init_mm.pgd when page fault happens. I
> expressed the page table that has not updated yet as old. For this
> reason, paging can lead to different result for differnet CPU.
Yes, paging can lead to different result for differnet CPU.
But the paging purpose is: allow the user debug kernel by
using gdb to process the core file. If the user want to debug
the user process in the core file, he can get the user process's
core from vmcore by using crash's extend gcore module.
>
>>> all the page tables referenced by all CPUs, we face inconsistency of
>>> some of the page tables. Essentially, we cannot avoid the issue that
>>> we see the page table older than the actual even if we use only one
>>> page table, but if restricting the use of page table to just one, we
>>> can at least avoid the inconsistency of multiple page tables. In other
>>> words, we can do paging processing normally though the table might be
>>> old.
>>>
>>> So, I think
>>> - using page tables for all the CPUs at the same time is problematic.
>>> - using only one page table of the exsiting CPUs is still safe.
>>>
>>> How about the code like this?
>>>
>>> cpu = find_cpu_paging_enabled(env);
>>
>> If there are more than two cpu's paging is enabled, which cpu should be
>> choosed?
>> We cannot say which one is better than another one.
>>
>
> I think so too. But now it sees
Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one
Quoting Anthony Liguori (anth...@codemonkey.ws): > On 03/26/2012 10:13 AM, Serge E. Hallyn wrote: > >Currently, if the user doesn't pass a uuid, the system uuid is set to > >all zeros. This patch generates a random one instead. > > > >Is there a reason to prefer all zeros? If not, can a patch like this > >one be applied? > > > >Signed-off-by: Serge Hallyn > > The other hypervisors don't have a concept of a transient guest like QEMU > does. True, and I generally don't like arguments of the form "but (some other) does it like this," but... > There is no state preserved between invocations of QEMU. Right, and I really don't know how many users (besides me) run kvm by hand, as opposed to using libvirt or testdrive. Perhaps testdrive should be extended. > Setting a random UUID doesn't seem like the right answer to me as it > would potentially break Windows VMs. > > Perhaps if the DMI UUID isn't set, you could look at the root filesystem's > UUID? Interesting idea. I don't know enough to know at which point in the code qemu would know that, but I can take a look. > Not all platforms have a notion of platform UUID so as Ubuntu > supports more architectures, this problem would have to be dealt > with eventually. So it sounds like in this form certainly it's nacked :) thanks, -serge
[Qemu-devel] [PATCH 05/12 v11] Add API to get memory mapping
Add API to get all virtual address and physical address mapping.
If the guest doesn't use paging, the virtual address is equal to the phyical
address. The virtual address and physical address mapping is for gdb's user, and
it does not include the memory that is not referenced by the page table. So if
you want to use crash to anaylze the vmcore, please do not specify -p option.
the reason why the -p option is not default explicitly: guest machine in a
catastrophic state can have corrupted memory, which we cannot trust.
Signed-off-by: Wen Congyang
---
address hatayama's comment
memory_mapping.c | 47 +++
memory_mapping.h | 15 +++
2 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/memory_mapping.c b/memory_mapping.c
index 718f271..627397a 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -164,3 +164,50 @@ void memory_mapping_list_init(MemoryMappingList *list)
list->last_mapping = NULL;
QTAILQ_INIT(&list->head);
}
+
+#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
+
+static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
+{
+CPUArchState *env;
+
+for (env = start_cpu; env != NULL; env = env->next_cpu) {
+if (cpu_paging_enabled(env)) {
+return env;
+}
+}
+
+return NULL;
+}
+
+int qemu_get_guest_memory_mapping(MemoryMappingList *list)
+{
+CPUArchState *env, *first_paging_enabled_cpu;
+RAMBlock *block;
+ram_addr_t offset, length;
+int ret;
+
+first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu);
+if (first_paging_enabled_cpu) {
+for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu)
{
+ret = cpu_get_memory_mapping(list, env);
+if (ret < 0) {
+return -1;
+}
+}
+return 0;
+}
+
+/*
+ * If the guest doesn't use paging, the virtual address is equal to
physical
+ * address.
+ */
+QLIST_FOREACH(block, &ram_list.blocks, next) {
+offset = block->offset;
+length = block->length;
+create_new_memory_mapping(list, offset, offset, length);
+}
+
+return 0;
+}
+#endif
diff --git a/memory_mapping.h b/memory_mapping.h
index 836b047..4d44641 100644
--- a/memory_mapping.h
+++ b/memory_mapping.h
@@ -44,4 +44,19 @@ void memory_mapping_list_free(MemoryMappingList *list);
void memory_mapping_list_init(MemoryMappingList *list);
+/*
+ * Return value:
+ *0: success
+ * -1: failed
+ * -2: unsupported
+ */
+#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
+int qemu_get_guest_memory_mapping(MemoryMappingList *list);
+#else
+static inline int qemu_get_guest_memory_mapping(MemoryMappingList *list)
+{
+return -2;
+}
+#endif
+
#endif
--
1.7.1
[Qemu-devel] [PATCH] Better support for dma_addr_t variables
A while back, we introduced the dma_addr_t type, which is supposed to
be used for bus visible memory addresses. At present, this is an
alias for target_phys_addr_t, but this will change when we eventually
add support for guest visible IOMMUs.
There are some instances of target_phys_addr_t in the code now which
should really be dma_addr_t, but can't be trivially converted due to
missing features which this patch corrects.
* We add DMA_ADDR_BITS analagous to TARGET_PHYS_ADDR_BITS. This is
important where we need to make a compile-time (#if) based on the
size of dma_addr_t.
* We add a new helper macro to create device properties which take a
dma_addr_t, currently an alias to DEFINE_PROP_TADDR().
Signed-off-by: David Gibson
---
dma.h |1 +
hw/qdev-dma.h |4
2 files changed, 5 insertions(+), 0 deletions(-)
create mode 100644 hw/qdev-dma.h
diff --git a/dma.h b/dma.h
index 05ac325..463095c 100644
--- a/dma.h
+++ b/dma.h
@@ -32,6 +32,7 @@ struct QEMUSGList {
#if defined(TARGET_PHYS_ADDR_BITS)
typedef target_phys_addr_t dma_addr_t;
+#define DMA_ADDR_BITS TARGET_PHYS_ADDR_BITS
#define DMA_ADDR_FMT TARGET_FMT_plx
struct ScatterGatherEntry {
diff --git a/hw/qdev-dma.h b/hw/qdev-dma.h
new file mode 100644
index 000..e407771
--- /dev/null
+++ b/hw/qdev-dma.h
@@ -0,0 +1,4 @@
+#include "qdev-addr.h"
+
+#define DEFINE_PROP_DMAADDR(_n, _s, _f, _d) \
+DEFINE_PROP_TADDR(_n, _s, _f, _d)
--
1.7.9.1
[Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
Currently dma_bdrv_io() takes a 'to_dev' boolean parameter to
determine the direction of DMA it is emulating. We already have a
DMADirection enum designed specifically to encode DMA directions.
This patch uses it for dma_bdrv_io() as well. This involves removing
the DMADirection definition from the #ifdef it was inside, but since that
only existed to protect the definition of dma_addr_t from places where
config.h is not included, there wasn't any reason for it to be there in
the first place.
Cc: Kevin Wolf
Signed-off-by: David Gibson
---
dma-helpers.c | 20
dma.h | 12 ++--
hw/ide/core.c |3 ++-
hw/ide/macio.c |3 ++-
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index c29ea6d..5f19a85 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -42,7 +42,7 @@ typedef struct {
BlockDriverAIOCB *acb;
QEMUSGList *sg;
uint64_t sector_num;
-bool to_dev;
+DMADirection dir;
bool in_cancel;
int sg_cur_index;
dma_addr_t sg_cur_byte;
@@ -76,7 +76,8 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
for (i = 0; i < dbs->iov.niov; ++i) {
cpu_physical_memory_unmap(dbs->iov.iov[i].iov_base,
- dbs->iov.iov[i].iov_len, !dbs->to_dev,
+ dbs->iov.iov[i].iov_len,
+ dbs->dir != DMA_DIRECTION_TO_DEVICE,
dbs->iov.iov[i].iov_len);
}
qemu_iovec_reset(&dbs->iov);
@@ -123,7 +124,8 @@ static void dma_bdrv_cb(void *opaque, int ret)
while (dbs->sg_cur_index < dbs->sg->nsg) {
cur_addr = dbs->sg->sg[dbs->sg_cur_index].base + dbs->sg_cur_byte;
cur_len = dbs->sg->sg[dbs->sg_cur_index].len - dbs->sg_cur_byte;
-mem = cpu_physical_memory_map(cur_addr, &cur_len, !dbs->to_dev);
+mem = cpu_physical_memory_map(cur_addr, &cur_len,
+ dbs->dir != DMA_DIRECTION_TO_DEVICE);
if (!mem)
break;
qemu_iovec_add(&dbs->iov, mem, cur_len);
@@ -170,11 +172,11 @@ static AIOPool dma_aio_pool = {
BlockDriverAIOCB *dma_bdrv_io(
BlockDriverState *bs, QEMUSGList *sg, uint64_t sector_num,
DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
-void *opaque, bool to_dev)
+void *opaque, DMADirection dir)
{
DMAAIOCB *dbs = qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
-trace_dma_bdrv_io(dbs, bs, sector_num, to_dev);
+trace_dma_bdrv_io(dbs, bs, sector_num, (dir == DMA_DIRECTION_TO_DEVICE));
dbs->acb = NULL;
dbs->bs = bs;
@@ -182,7 +184,7 @@ BlockDriverAIOCB *dma_bdrv_io(
dbs->sector_num = sector_num;
dbs->sg_cur_index = 0;
dbs->sg_cur_byte = 0;
-dbs->to_dev = to_dev;
+dbs->dir = dir;
dbs->io_func = io_func;
dbs->bh = NULL;
qemu_iovec_init(&dbs->iov, sg->nsg);
@@ -195,14 +197,16 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void
*opaque)
{
-return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, false);
+return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque,
+ DMA_DIRECTION_FROM_DEVICE);
}
BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void
*opaque)
{
-return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, true);
+return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque,
+ DMA_DIRECTION_TO_DEVICE);
}
diff --git a/dma.h b/dma.h
index 20e86d2..05ac325 100644
--- a/dma.h
+++ b/dma.h
@@ -17,6 +17,11 @@
typedef struct ScatterGatherEntry ScatterGatherEntry;
+typedef enum {
+DMA_DIRECTION_TO_DEVICE = 0,
+DMA_DIRECTION_FROM_DEVICE = 1,
+} DMADirection;
+
struct QEMUSGList {
ScatterGatherEntry *sg;
int nsg;
@@ -29,11 +34,6 @@ typedef target_phys_addr_t dma_addr_t;
#define DMA_ADDR_FMT TARGET_FMT_plx
-typedef enum {
-DMA_DIRECTION_TO_DEVICE = 0,
-DMA_DIRECTION_FROM_DEVICE = 1,
-} DMADirection;
-
struct ScatterGatherEntry {
dma_addr_t base;
dma_addr_t len;
@@ -51,7 +51,7 @@ typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs,
int64_t sector_num,
BlockDriverAIOCB *dma_bdrv_io(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector_num,
DMAIOFunc *io_func, BlockDriverCompletionFunc
*cb,
- void *opaque, bool to_dev);
+ void *opaque, DMADirection dir);
BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
BlockDriverCompletionFunc *cb, void *opaque);
diff --git a/hw/
Re: [Qemu-devel] [PATCH 10/12 v11] make gdb_id() generally avialable and rename it to cpu_index()
From: Wen Congyang
Subject: [PATCH 10/12 v11] make gdb_id() generally avialable and rename it to
cpu_index()
Date: Mon, 26 Mar 2012 18:05:39 +0800
>
> -static inline int gdb_id(CPUArchState *env)
> -{
> -#if defined(CONFIG_USER_ONLY) && defined(CONFIG_USE_NPTL)
> -return env->host_tid;
> -#else
> -return env->cpu_index + 1;
> -#endif
> -}
> -
I meant in gdbstub.c:
static inline int gdb_id(CPUArchState *env)
{
return cpu_index(env);
}
Thanks.
HATAYAMA, Daisuke
Re: [Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest
On 03/27/2012 06:10 AM, Anthony Liguori wrote: On 03/16/2012 03:54 AM, Jason Wang wrote: This an update of series that let guest and qemu to be co-operated to send gratuitous packets when needed such as after migration, loadvm and continuing. As it's hard for qemu to track the network configuration in guest such as bondings, vlans or ipv6. So current gratuitous may not work under those situations. Can you be more specific about the failure scenarios? The failure can happen when: - Guest does not use primary mac address. Current qemu only send rarp packets for primary mac address. This looks could be solved by iterating nic with mac address table, but their size are limited and guest could use all-uni/promisc mode to use more mac addresses. So it's almost impossible to track all addresses in qemu side. - Guest have some network configuration such as 802.1Q vlan, in this case, we need to send tagged gratuitous packet which qemu can't handle. The point is qemu does not know the network configuration in guest or how mac addresses are used, so it's better for us to let guest choose the correct way to do this if they can. If guest could not do the announcement, the rarp packets would be sent as in the past. Does this mean that migration cannot work today with guests using ipv6? I don't think just pushing this to the guest is an acceptable solution in the short term. I haven't check, but w/o this patch a ipv4 rarp would be sent even guest are using ipv6; w/ this patch, a ipv6 neighbor advertisement would be sent by guest which looks pretty reasonable. Did you see any drawbacks of this method? Xen and hyper-v also let guest to send gratuitous packet for their para-virtualized network card. Are there scenarios we cannot handle no matter what in the host? Does this mean that for emulated drivers, we're completely out of luck? A possible improvement but not a final solution is to send garp for all address in the mac table I think. Regards, Anthony Liguori
[Qemu-devel] [PATCH]booke:Use MMU API for creating initial mapping for secondary cpus
Initial Mapping creation for secondary CPU in SMP was missing new MMU API. Signed-off-by: Bharat Bhushan --- hw/ppce500_spin.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c index 6b8a189..e8cf154 100644 --- a/hw/ppce500_spin.c +++ b/hw/ppce500_spin.c @@ -86,6 +86,7 @@ static void mmubooke_create_initial_mapping(CPUState *env, tlb->mas2 = (va & TARGET_PAGE_MASK) | MAS2_M; tlb->mas7_3 = pa & TARGET_PAGE_MASK; tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX; +env->tlb_dirty = true; } static void spin_kick(void *data) -- 1.7.0.4
Re: [Qemu-devel] [PATCH] block: add the support to drain throttled requests
On Mon, Mar 26, 2012 at 10:21 PM, Stefan Hajnoczi wrote:
> On Tue, Mar 20, 2012 at 11:44 AM, Stefan Hajnoczi
> wrote:
>> On Tue, Mar 20, 2012 at 10:58:10AM +0100, Kevin Wolf wrote:
>>> Am 20.03.2012 10:47, schrieb Paolo Bonzini:
>>> > Il 20/03/2012 10:40, Zhi Yong Wu ha scritto:
>>> >> HI, Kevin,
>>> >>
>>> >> We hope that I/O throttling can be shipped without known issue in QEMU
>>> >> 1.1, so if you are available, can you give this patch some love?
>>> >
>>> > I'm sorry to say this, but I think I/O throttling is impossible to save.
>>> > As it is implemented now, it just cannot work in the presence of
>>> > synchronous I/O, except at the cost of busy waiting with the global
>>> > mutex taken. See the message from Stefan yesterday.
>>>
>>> qemu_aio_flush() is busy waiting with the global mutex taken anyway, so
>>> it doesn't change that much.
>>
>> Yesterday I only posted an analysis of the bug but here are some
>> thoughts on how to move forward. Throttling itself is not the problem.
>> We've known that synchronous operations in the vcpu thread are a problem
>> long before throttling. This is just another reason to convert device
>> emulation to use asynchronous interfaces.
>>
>> Here is the list of device models that perform synchronous block I/O:
>> hw/fdc.c
>> hw/ide/atapi.c
>> hw/ide/core.c
>> hw/nand.c
>> hw/onenand.c
>> hw/pflash_cfi01.c
>> hw/pflash_cfi02.c
>> hw/sd.c
>>
>> Zhi Hui Li is working on hw/fdc.c and recently sent a patch.
>>
>> I think it's too close to QEMU 1.1 to convert all the remaining devices
>> and test them properly before the soft-freeze. But it's probably
>> possible to convert IDE before the soft-freeze.
>>
>> In the meantime we could add this to bdrv_rw_co():
>>
>> if (bs->io_limits_enabled) {
>> fprintf(stderr, "Disabling I/O throttling on '%s' due "
>> "to synchronous I/O\n", bdrv_get_device_name(bs));
>> bdrv_io_limits_disable(bs);
>> }
>>
>> It's not pretty but tells the user there is an issue and avoids
>> deadlocking.
>
> No one has commented on this suggestion. I think leaving a known hang
> in QEMU 1.1 is undesirable. Better to have this warning and disable
> throttling in the case we cannot support right now.
IDE is using both sync API and async API when guest boot.
>
> Kevin: Would you accept a patch like this? Or do you have another
> solution in mind?
>
> Stefan
--
Regards,
Zhi Yong Wu
[Qemu-devel] [PATCH 1/2] block: add the support to drain throttled requests
From: Zhi Yong Wu
Signed-off-by: Zhi Yong Wu
---
block.c | 16 +++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/block.c b/block.c
index b88ee90..1fbf4dd 100644
--- a/block.c
+++ b/block.c
@@ -862,8 +862,22 @@ void bdrv_close_all(void)
void bdrv_drain_all(void)
{
BlockDriverState *bs;
+bool busy;
-qemu_aio_flush();
+do {
+busy = false;
+qemu_aio_flush();
+
+/* FIXME: We do not have timer support here, so this is effectively
+ * a busy wait.
+ */
+QTAILQ_FOREACH(bs, &bdrv_states, list) {
+if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
+qemu_co_queue_restart_all(&bs->throttled_reqs);
+busy = true;
+}
+}
+} while (busy);
/* If requests are still pending there is a bug somewhere */
QTAILQ_FOREACH(bs, &bdrv_states, list) {
--
1.7.6
[Qemu-devel] [PATCH 2/2] block: disable I/O throttling on sync api
From: Zhi Yong Wu
Signed-off-by: Zhi Yong Wu
---
block.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/block.c b/block.c
index 1fbf4dd..5baf340 100644
--- a/block.c
+++ b/block.c
@@ -1477,6 +1477,12 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t
sector_num, uint8_t *buf,
qemu_iovec_init_external(&qiov, &iov, 1);
+if (bs->io_limits_enabled) {
+fprintf(stderr, "Disabling I/O throttling on '%s' due "
+"to synchronous I/O.\n", bdrv_get_device_name(bs));
+bdrv_io_limits_disable(bs);
+}
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_rw_co_entry(&rwco);
--
1.7.6
Re: [Qemu-devel] [PATCH 1/2] Isolation groups
On Wed, Mar 21, 2012 at 03:12:58PM -0600, Alex Williamson wrote:
> On Sat, 2012-03-17 at 15:57 +1100, David Gibson wrote:
> > On Fri, Mar 16, 2012 at 01:31:18PM -0600, Alex Williamson wrote:
> > > On Fri, 2012-03-16 at 14:45 +1100, David Gibson wrote:
> > > > On Thu, Mar 15, 2012 at 02:15:01PM -0600, Alex Williamson wrote:
> > > > > On Wed, 2012-03-14 at 20:58 +1100, David Gibson wrote:
> > > > > > On Tue, Mar 13, 2012 at 10:49:47AM -0600, Alex Williamson wrote:
> > > > > > > On Wed, 2012-03-14 at 01:33 +1100, David Gibson wrote:
> > > > > > > > On Mon, Mar 12, 2012 at 04:32:54PM -0600, Alex Williamson wrote:
> > > > > > > > > +/*
> > > > > > > > > + * Add a device to an isolation group. Isolation groups
> > > > > > > > > start empty and
> > > > > > > > > + * must be told about the devices they contain. Expect this
> > > > > > > > > to be called
> > > > > > > > > + * from isolation group providers via notifier.
> > > > > > > > > + */
> > > > > > > >
> > > > > > > > Doesn't necessarily have to be from a notifier, particularly if
> > > > > > > > the
> > > > > > > > provider is integrated into host bridge code.
> > > > > > >
> > > > > > > Sure, a provider could do this on it's own if it wants. This just
> > > > > > > provides some infrastructure for a common path. Also note that
> > > > > > > this
> > > > > > > helps to eliminate all the #ifdef CONFIG_ISOLATION in the
> > > > > > > provider. Yet
> > > > > > > to be seen whether that can reasonably be the case once isolation
> > > > > > > groups
> > > > > > > are added to streaming DMA paths.
> > > > > >
> > > > > > Right, but other than the #ifdef safety, which could be achieved
> > > > > > more
> > > > > > simply, I'm not seeing what benefit the infrastructure provides over
> > > > > > directly calling the bus notifier function. The infrastructure
> > > > > > groups
> > > > > > the notifiers by bus type internally, but AFAICT exactly one bus
> > > > > > notifier call would become exactly one isolation notifier call, and
> > > > > > the notifier callback itself would be almost identical.
> > > > >
> > > > > I guess I don't see this as a fundamental design point of the
> > > > > proposal,
> > > > > it's just a convenient way to initialize groups as a side-band
> > > > > addition
> > > > > until isolation groups become a more fundamental part of the iommu
> > > > > infrastructure. If you want to do that level of integration in your
> > > > > provider, do it and make the callbacks w/o the notifier. If nobody
> > > > > ends
> > > > > up using them, we'll remove them. Maybe it will just end up being a
> > > > > bootstrap. In the typical case, yes, one bus notifier is one
> > > > > isolation
> > > > > notifier. It does however also allow one bus notifier to become
> > > > > multiple isolation notifiers, and includes some filtering that would
> > > > > just be duplicated if every provider decided to implement their own
> > > > > bus
> > > > > notifier.
> > > >
> > > > Uh.. I didn't notice any filtering? That's why I'm asking.
> > >
> > > Not much, but a little:
> > >
> > > + switch (action) {
> > > + case BUS_NOTIFY_ADD_DEVICE:
> > > + if (!dev->isolation_group)
> > > + blocking_notifier_call_chain(¬ifier->notifier,
> > > + ISOLATION_NOTIFY_ADD_DEVICE, dev);
> > > + break;
> > > + case BUS_NOTIFY_DEL_DEVICE:
> > > + if (dev->isolation_group)
> > > + blocking_notifier_call_chain(¬ifier->notifier,
> > > + ISOLATION_NOTIFY_DEL_DEVICE, dev);
> > > + break;
> > > + }
T> >
> >
> > Ah, I see, fair enough.
> >
> > A couple of tangential observations. First, I suspect using
> > BUS_NOTIFY_DEL_DEVICE is a very roundabout way of handling hot-unplug,
> > it might be better to have an unplug callback in the group instead.
>
> There's really one already. Assuming the device is attached to a group
> driver, the .remove entry point on the driver will get called first. It
> doesn't get to return error, but it can block.
Hrm. Assuming the isolation provider has installed a driver for the
relevant bus type. And as we're discussing elsewhere, I think we have
situations where the groups will get members on (subordinate) busses
which the isolation provider doesn't have explicit knowledge of.
> The DEL_DEVICE will only
> happen after the group driver has relinquished the device, or at least
> returned from remove(). For a device with no driver, the group would
> only learn about it through the notifier, but it probably doesn't need
> anything more direct.
DEL_DEVICE is certainly sufficient, it just seems a bit unnecessarily
awkward.
> > Second, I don't think aborting the call chain early for hot-plug is
> > actually a good idea. I can't see a clear guarantee on the order, so
> > individual providers couldn't rely on that short-cut behaviour. Whi
[Qemu-devel] [PATCH v4] w32: Support tests (make check)
Adding $(EXESUF) is needed to make those tests work on w32 hosts, too. v2: Rebased, added new tests, tests sorted alphabetically. v3: Rebased, $(EXESUF) for qemu-img, qemu-io which were recently added. v4: Rebased, new test test-qmp-commands which was recently added. Cc: Anthony Liguori Cc: Kevin Wolf Cc: Andreas Färber Reviewed-by: Andreas Färber Signed-off-by: Stefan Weil --- This is the 4rd version of my patch. The 1st was sent in February, and again some other newer (!) patches were committed, so I had to rebase my patch a third time now. Anthony, please commit. Cheers, Stefan Weil tests/Makefile | 42 +- 1 files changed, 25 insertions(+), 17 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 94ea342..ca03b3a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,20 +1,28 @@ export SRC_PATH -CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist -CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor -CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine -CHECKS += test-qmp-commands +CHECKS = check-qdict$(EXESUF) +CHECKS += check-qfloat$(EXESUF) +CHECKS += check-qint$(EXESUF) +CHECKS += check-qjson$(EXESUF) +CHECKS += check-qlist$(EXESUF) +CHECKS += check-qstring$(EXESUF) +CHECKS += test-coroutine$(EXESUF) +CHECKS += test-qmp-commands$(EXESUF) +CHECKS += test-qmp-input-visitor$(EXESUF) +CHECKS += test-qmp-output-visitor$(EXESUF) +CHECKS += test-string-input-visitor$(EXESUF) +CHECKS += test-string-output-visitor$(EXESUF) CHECKS += $(SRC_PATH)/tests/qemu-iotests-quick.sh check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o: $(GENERATED_HEADERS) -check-qint: check-qint.o qint.o $(tools-obj-y) -check-qstring: check-qstring.o qstring.o $(tools-obj-y) -check-qdict: check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qlist.o $(tools-obj-y) -check-qlist: check-qlist.o qlist.o qint.o $(tools-obj-y) -check-qfloat: check-qfloat.o qfloat.o $(tools-obj-y) -check-qjson: check-qjson.o $(qobject-obj-y) $(tools-obj-y) -test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(tools-obj-y) +check-qint$(EXESUF): check-qint.o qint.o $(tools-obj-y) +check-qstring$(EXESUF): check-qstring.o qstring.o $(tools-obj-y) +check-qdict$(EXESUF): check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qlist.o $(tools-obj-y) +check-qlist$(EXESUF): check-qlist.o qlist.o qint.o $(tools-obj-y) +check-qfloat$(EXESUF): check-qfloat.o qfloat.o $(tools-obj-y) +check-qjson$(EXESUF): check-qjson.o $(qobject-obj-y) $(tools-obj-y) +test-coroutine$(EXESUF): test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(tools-obj-y) test-qmp-input-visitor.o test-qmp-output-visitor.o \ test-string-input-visitor.o test-string-output-visitor.o \ @@ -32,21 +40,21 @@ $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py test-string-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) -test-string-output-visitor: test-string-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o +test-string-output-visitor$(EXESUF): test-string-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o test-string-input-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) -test-string-input-visitor: test-string-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o +test-string-input-visitor$(EXESUF): test-string-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o test-qmp-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) -test-qmp-output-visitor: test-qmp-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o +test-qmp-output-visitor$(EXESUF): test-qmp-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o test-qmp-input-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) -test-qmp-input-visitor: test-qmp-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o +test-qmp-input-visitor$(EXESUF): test-qmp-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.
[Qemu-devel] [PATCH 1/2] block: add the support to drain throttled requests
From: Zhi Yong Wu
Signed-off-by: Paolo Bonzini
Signed-off-by: Zhi Yong Wu
---
block.c | 16 +++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/block.c b/block.c
index b88ee90..1fbf4dd 100644
--- a/block.c
+++ b/block.c
@@ -862,8 +862,22 @@ void bdrv_close_all(void)
void bdrv_drain_all(void)
{
BlockDriverState *bs;
+bool busy;
-qemu_aio_flush();
+do {
+busy = false;
+qemu_aio_flush();
+
+/* FIXME: We do not have timer support here, so this is effectively
+ * a busy wait.
+ */
+QTAILQ_FOREACH(bs, &bdrv_states, list) {
+if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
+qemu_co_queue_restart_all(&bs->throttled_reqs);
+busy = true;
+}
+}
+} while (busy);
/* If requests are still pending there is a bug somewhere */
QTAILQ_FOREACH(bs, &bdrv_states, list) {
--
1.7.6
[Qemu-devel] [PATCH 2/2] block: disable I/O throttling on sync api
From: Zhi Yong Wu
Signed-off-by: Stefan Hajnoczi
Signed-off-by: Zhi Yong Wu
---
block.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/block.c b/block.c
index 1fbf4dd..5baf340 100644
--- a/block.c
+++ b/block.c
@@ -1477,6 +1477,12 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t
sector_num, uint8_t *buf,
qemu_iovec_init_external(&qiov, &iov, 1);
+if (bs->io_limits_enabled) {
+fprintf(stderr, "Disabling I/O throttling on '%s' due "
+"to synchronous I/O.\n", bdrv_get_device_name(bs));
+bdrv_io_limits_disable(bs);
+}
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_rw_co_entry(&rwco);
--
1.7.6
Re: [Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest
On Mon, Mar 26, 2012 at 05:10:15PM -0500, Anthony Liguori wrote: > On 03/16/2012 03:54 AM, Jason Wang wrote: > >This an update of series that let guest and qemu to be co-operated to > >send gratuitous packets when needed such as after migration, loadvm > >and continuing. > > > >As it's hard for qemu to track the network configuration in guest such > >as bondings, vlans or ipv6. So current gratuitous may not work under > >those situations. > > Can you be more specific about the failure scenarios? > > Does this mean that migration cannot work today with guests using > ipv6? I don't think just pushing this to the guest is an acceptable > solution in the short term. > > Are there scenarios we cannot handle no matter what in the host? Consider a nested virt scenario. It seems clear that you either must notify the guest about migration, or learn nested guest macs. > Does this mean that for emulated drivers, we're completely out of > luck? > > Regards, > > Anthony Liguori For these, I think we can cause announcements by sending link up event to the guest. -- MST
