[Qemu-devel] Re: [PATCH] QEMUFileBuffered: indicate that we're ready when the underlying file is ready
On 07/07/2010 07:44 PM, Avi Kivity wrote: QEMUFileBuffered stops writing when the underlying QEMUFile is not ready, and tells its producer so. However, when the underlying QEMUFile becomes ready, it neglects to pass that information along, resulting in stoppage of all data until the next tick (a tenths of a second). Usually this doesn't matter, because most QEMUFiles used with QEMUFileBuffered are almost always ready, but in the case of exec: migration this is not true, due to the small pipe buffers used to connect to the target process. The result is very slow migration. Fix by detecting the readiness notification and propagating it. The detection is a little ugly since QEMUFile overloads put_buffer() to send it, but that's the suject for a different patch. Ping. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] [Tracing][PATCH] Add options to specify trace file name at startup and runtime.
This patch adds an optional command line switch '-trace' to specify the
filename to write traces to, when qemu starts.
Eg, If compiled with the 'simple' trace backend,
[t...@system]$ qemu -trace FILENAME IMAGE
Allows the binary traces to be written to FILENAME instead of the option
set at config-time.
Also, this adds monitor sub-command 'set' to trace-file commands to
dynamically change trace log file at runtime.
Eg,
(qemu)trace-file set FILENAME
This allows one to set trace outputs to FILENAME from the default
specified at startup.
Signed-off-by: Prerna Saxena
---
monitor.c |6 ++
qemu-monitor.hx |6 +++---
qemu-options.hx | 11 +++
simpletrace.c | 41 -
tracetool |1 +
vl.c| 22 ++
6 files changed, 75 insertions(+), 12 deletions(-)
diff --git a/monitor.c b/monitor.c
index 1e35a6b..8e2a3a6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -544,6 +544,7 @@ static void do_change_trace_event_state(Monitor *mon, const
QDict *qdict)
static void do_trace_file(Monitor *mon, const QDict *qdict)
{
const char *op = qdict_get_try_str(qdict, "op");
+const char *arg = qdict_get_try_str(qdict, "arg");
if (!op) {
st_print_trace_file_status((FILE *)mon, &monitor_fprintf);
@@ -553,8 +554,13 @@ static void do_trace_file(Monitor *mon, const QDict *qdict)
st_set_trace_file_enabled(false);
} else if (!strcmp(op, "flush")) {
st_flush_trace_buffer();
+} else if (!strcmp(op, "set")) {
+if (arg) {
+st_set_trace_file(arg);
+}
} else {
monitor_printf(mon, "unexpected argument \"%s\"\n", op);
+monitor_printf(mon, "Options are: [on | off| flush| set FILENAME]");
}
}
#endif
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 25887bd..adfaf2b 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -276,9 +276,9 @@ ETEXI
{
.name = "trace-file",
-.args_type = "op:s?",
-.params = "op [on|off|flush]",
-.help = "open, close, or flush trace file",
+.args_type = "op:s?,arg:F?",
+.params = "on|off|flush|set [arg]",
+.help = "open, close, or flush trace file, or set a new file
name",
.mhandler.cmd = do_trace_file,
},
diff --git a/qemu-options.hx b/qemu-options.hx
index d1d2272..aea9675 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2223,6 +2223,17 @@ Normally QEMU loads a configuration file from
@var{sysconfdir}/qemu.conf and
@var{sysconfdir}/targ...@var{arch}.conf on startup. The @code{-nodefconfig}
option will prevent QEMU from loading these configuration files at startup.
ETEXI
+#ifdef CONFIG_SIMPLE_TRACE
+DEF("trace", HAS_ARG, QEMU_OPTION_trace,
+"-trace\n"
+"Specify a trace file to log traces to\n",
+QEMU_ARCH_ALL)
+STEXI
+...@item -trace
+...@findex -trace
+Specify a trace file to log output traces to.
+ETEXI
+#endif
HXCOMM This is the last statement. Insert new options before this line!
STEXI
diff --git a/simpletrace.c b/simpletrace.c
index 71110b3..5812fe9 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -20,25 +20,48 @@ enum {
static TraceRecord trace_buf[TRACE_BUF_LEN];
static unsigned int trace_idx;
static FILE *trace_fp;
-static bool trace_file_enabled = true;
+static char *trace_file_name = NULL;
+static bool trace_file_enabled = false;
void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE
*stream, const char *fmt, ...))
{
-stream_printf(stream, "Trace file \"" CONFIG_TRACE_FILE "\" %s.\n",
- getpid(), trace_file_enabled ? "on" : "off");
+stream_printf(stream, "Trace file \"%s\" %s.\n",
+ trace_file_name, trace_file_enabled ? "on" : "off");
}
static bool open_trace_file(void)
{
-char *filename;
+trace_fp = fopen(trace_file_name, "w");
+return trace_fp != NULL;
+}
-if (asprintf(&filename, CONFIG_TRACE_FILE, getpid()) < 0) {
-return false;
+/**
+ * set_trace_file : To set the name of a trace file.
+ * @file : pointer to the name to be set.
+ * If NULL, set to the default name- set at config time.
+ */
+bool st_set_trace_file(const char *file)
+{
+if (trace_file_enabled) {
+st_set_trace_file_enabled(false);
}
-trace_fp = fopen(filename, "w");
-free(filename);
-return trace_fp != NULL;
+if (trace_file_name) {
+free(trace_file_name);
+}
+
+if (!file) {
+if (asprintf(&trace_file_name, CONFIG_TRACE_FILE, getpid()) < 0) {
+ return false;
+}
+} else {
+if (asprintf(&trace_file_name, "%s", file) < 0) {
+return false;
+}
+}
+
+st_set_trace_file_enabled(true);
+return true;
}
static void flush_trace_file(void)
diff --git a/tracetool b/tracetool
index ac832af..5b979f5 100755
--- a/tracetool
+++ b/tracetool
@@ -158,6 +158,7 @@ void st_prin
[Qemu-devel] Re: [RFC PATCH 04/14] KVM-test: Add a new subtest ping
On Tue, Jul 27, 2010 at 10:15:49AM -0300, Lucas Meneghel Rodrigues wrote:
> On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
> > This test use ping to check the virtual nics, it contains two kinds of test:
> > 1. Packet loss ratio test, ping the guest with different size of packets.
> > 2. Stress test, flood ping guest then use ordinary ping to test the network.
> >
> > The interval and packet size could be configurated through tests_base.cfg
> >
> > Signed-off-by: Jason Wang
> > Signed-off-by: Amos Kong
> > ---
> > 0 files changed, 0 insertions(+), 0 deletions(-)
> >
> > diff --git a/client/tests/kvm/tests/ping.py b/client/tests/kvm/tests/ping.py
> > new file mode 100644
> > index 000..cfccda4
> > --- /dev/null
> > +++ b/client/tests/kvm/tests/ping.py
> > @@ -0,0 +1,71 @@
> > +import logging, time, re, commands
> > +from autotest_lib.client.common_lib import error
> > +import kvm_subprocess, kvm_test_utils, kvm_utils, kvm_net_utils
> > +
> > +
> > +def run_ping(test, params, env):
> > +"""
> > +Ping the guest with different size of packets.
> > +
> > +Packet Loss Test:
> > +1) Ping the guest with different size/interval of packets.
> > +Stress Test:
> > +1) Flood ping the guest.
> > +2) Check if the network is still usable.
> > +
> > +@param test: Kvm test object
> > +@param params: Dictionary with the test parameters
> > +@param env: Dictionary with test environment.
> > +"""
> > +
> > +vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> > +session = kvm_test_utils.wait_for_login(vm)
> > +
> > +counts = params.get("ping_counts", 100)
> > +flood_minutes = float(params.get("flood_minutes", 10))
> > +nics = params.get("nics").split()
> > +strict_check = params.get("strict_check", "no") == "yes"
> > +
> > +packet_size = [0, 1, 4, 48, 512, 1440, 1500, 1505, 4054, 4055, 4096,
> > 4192,
> > + 8878, 9000, 32767, 65507]
> > +
> > +try:
> > +for i, nic in enumerate(nics):
> > +ip = vm.get_address(i)
> > +if not ip:
> > +logging.error("Could not get the ip of nic index %d" % i)
> > +continue
> > +
> > +for size in packet_size:
> > +logging.info("Ping with packet size %s" % size)
> > +status, output = kvm_net_utils.ping(ip, 10,
> > +packetsize = size,
> > +timeout = 20)
> > +if strict_check:
> > +ratio = kvm_net_utils.get_loss_ratio(output)
> > +if ratio != 0:
> > +raise error.TestFail(" Loss ratio is %s for packet
> > size"
> > + " %s" % (ratio, size))
> > +else:
> > +if status != 0:
> > +raise error.TestFail(" Ping returns non-zero value
> > %s" %
> > + output)
>
> ^ "Ping failed, status: %s, output: %s" would be a better exception
> message.
>
> > +
> > +logging.info("Flood ping test")
> > +kvm_net_utils.ping(ip, None, flood = True, output_func= None,
> > + timeout = flood_minutes * 60)
>
> ^ Please get rid of the spaces here
>
> > +logging.info("Final ping test")
> > +status, output = kvm_net_utils.ping(ip, counts,
> > +timeout = float(counts) *
> > 1.5)
>
> ^ What is this last test for again?
We could not raise an error when flood ping failed(loss packet/ return
non-zero), it's too strict.
But we must check the ping result before/after floop-ping.
>
> > +if strict_check:
> > +ratio = kvm_net_utils.get_loss_ratio(output)
> > +if ratio != 0:
> > +raise error.TestFail("Packet loss ratio is %s after
> > flood"
> > + % ratio)
> > +else:
> > +if status != 0:
> > +raise error.TestFail(" Ping returns non-zero value %s"
> > %
> > + output)
> > +finally:
> > +session.close()
> > diff --git a/client/tests/kvm/tests_base.cfg.sample
> > b/client/tests/kvm/tests_base.cfg.sample
> > index 6710c00..4f58dc0 100644
> > --- a/client/tests/kvm/tests_base.cfg.sample
> > +++ b/client/tests/kvm/tests_base.cfg.sample
> > @@ -349,6 +349,11 @@ variants:
> > kill_vm_gracefully_vm2 = no
> > address_index_vm2 = 1
> >
> > +- ping: install setup unattended_install.cdrom
> > +type = ping
> > +counts = 100
> > +flood_minutes = 10
> > +
> > - physical_resources_check: install setup unattended_install.cdrom
> > type = physical_resources_check
> > catch_uuid_cmd = dmidecode | awk -F: '/UUI
Re: [Qemu-devel] [RFC PATCH 02/14] KVM Test: Add a function get_interface_name() to kvm_net_utils.py
On Wed, Jul 28, 2010 at 01:29:22PM +0300, Michael Goldish wrote:
> On 07/27/2010 05:08 AM, Lucas Meneghel Rodrigues wrote:
> > On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
> >> The function get_interface_name is used to get the interface name of linux
> >> guest through the macaddress of specified macaddress.
> >
> > I wonder if it wouldn't be overkill to have separate utility libraries
> > on the kvm test instead of a single kvm_utils and kvm_test_utils like
> > you are proposing. Any thoughts Michael?
>
> Yes, looks like this could be in kvm_test_utils.py, especially if
> there's only a small number of functions here.
Current functions of network are not too much, but we may add a lot of new
utility functions in the future.
It's more clear to use another file.
> >> Signed-off-by: Jason Wang
> >> Signed-off-by: Amos Kong
> >> ---
> >> 0 files changed, 0 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/client/tests/kvm/kvm_net_utils.py
> >> b/client/tests/kvm/kvm_net_utils.py
> >> new file mode 100644
> >> index 000..ede4965
> >> --- /dev/null
> >> +++ b/client/tests/kvm/kvm_net_utils.py
> >> @@ -0,0 +1,18 @@
> >> +import re
> >> +
> >> +def get_linux_ifname(session, mac_address):
> >> +"""
> >> +Get the interface name through the mac address.
> >> +
> >> +@param session: session to the virtual machine
> >> +@mac_address: the macaddress of nic
> >> +"""
> >> +
> >> +output = session.get_command_output("ifconfig -a")
> >> +
> >> +try:
> >> +ethname = re.findall("(\w+)\s+Link.*%s" % mac_address, output,
> >> + re.IGNORECASE)[0]
> >> +return ethname
> >> +except:
> >> +return None
> >>
> >>
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Re: [Qemu-devel] [RFC PATCH 01/14] KVM-test: Add a new macaddress pool algorithm
On Tue, Jul 20, 2010 at 06:53:27PM +0300, Michael Goldish wrote:
> On 07/20/2010 04:44 PM, Amos Kong wrote:
> > On Tue, Jul 20, 2010 at 01:19:39PM +0300, Michael Goldish wrote:
> >>
> >
> > Michael,
> >
> > Thanks for your comments. Let's simplify this method together.
I would produce a v2 later basing on our discussion.
> >>> +def get_ifname(self, nic_index=0):
> >>> +"""
> >>> +Return the ifname of tap device for the guest nic.
> >>> +
> >>> +@param nic_index: Index of the NIC
> >>> +"""
> >>> +
> >>> +nics = kvm_utils.get_sub_dict_names(self.params, "nics")
> >>> +nic_name = nics[nic_index]
> >>> +nic_params = kvm_utils.get_sub_dict(self.params, nic_name)
> >>> +if nic_params.get("nic_ifname"):
> >>> +return nic_params.get("nic_ifname")
> >>> +else:
> >>> +return "%s_%s_%s" % (nic_params.get("nic_model"),
> >>> + nic_index, self.vnc_port)
> >>
> >> What's the purpose of this string?
> >
> > Just avoid repeated ifname. The vnc_port is unique for each VM, nic_index
> > is unique for each nic of one VM.
>
> self.instance should also be unique, though it's quite long.
qemu can only receive 15 chars as ifname, instance is too long.
if the prefix of two instance are same, the actual ifnames will not be unique.
linux-user/syscall.c:
#define IFNAMSIZ16
net/tap-linux.c:
int tap_open(...
...
if (ifname[0] != '\0')
pstrcpy(ifr.ifr_name, IFNAMSIZ, ifname);
else
pstrcpy(ifr.ifr_name, IFNAMSIZ, "tap%d");
[Qemu-devel] Re: KVM call agenda for August 3
On Mon, Aug 2, 2010 at 8:46 PM, Juan Quintela wrote: > > Please send in any agenda items you are interested in covering. > I would like to briefly RFC about some snapshot issues that I have being dealing, for the conversion of savevm/loadvm to QMP. They are listed here: http://wiki.qemu.org/Features/SnapshottingImprovements Regards, Miguel
[Qemu-devel] KVM call agenda for August 3
Please send in any agenda items you are interested in covering. thanks, Juan
[Qemu-devel] Re: [PATCH] [sparc32] fix last cpu timer initialization
Btw, it would be nice to get this patch applied for 0.13: it's a pure
fix, and it allows running the OBP v2.10 for LX and some older OBP
versions for SS-20 too.
2010/8/2 Blue Swirl :
> Thanks, applied. Please remember to use Signed-off-by tag.
Grrr. Git 1.6.2.5 seems to ignore the option signoff = true in the
config file. I'll update my git right now.
>
>
> On Mon, Aug 2, 2010 at 5:58 PM, Artyom Tarasenko
> wrote:
>> The timer #0 is the system timer, so the timer #num_cpu is the
>> timer of the last CPU, and it must be initialized in slavio_timer_reset.
>>
>> Don't mark non-existing timers as running.
>> ---
>> hw/slavio_timer.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/slavio_timer.c b/hw/slavio_timer.c
>> index d787553..c125de4 100644
>> --- a/hw/slavio_timer.c
>> +++ b/hw/slavio_timer.c
>> @@ -377,12 +377,12 @@ static void slavio_timer_reset(DeviceState *d)
>> curr_timer->limit = 0;
>> curr_timer->count = 0;
>> curr_timer->reached = 0;
>> - if (i < s->num_cpus) {
>> + if (i <= s->num_cpus) {
>> ptimer_set_limit(curr_timer->timer,
>> LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
>> ptimer_run(curr_timer->timer, 0);
>> + curr_timer->running = 1;
>> }
>> - curr_timer->running = 1;
>> }
>> s->cputimer_mode = 0;
>> }
>> --
>> 1.6.2.5
>>
>>
>
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
[Qemu-devel] Re: [PATCH] e1000: Fix hotplug
rtl8139 has the same problem, except there's a much bigger pile of code in rtl8139_reset()? I think maybe we need to revisit this wholesale remove of reset calls from init functions, unless I'm missing how hotplug is supposed to work. Thanks, Alex On Mon, 2010-08-02 at 15:15 -0600, Alex Williamson wrote: > When we removed the call to e1000_reset() back in cset c1699988, we > left some register state uninitialized. When we hotplug the device, > we don't go through a reset cycle, which means a hot added e1000 is > useless until the VM reboots. Duplicate the bits we need from > e1000_reset(). > > Signed-off-by: Alex Williamson > --- > > 0.13 candidate? > > hw/e1000.c |4 > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/hw/e1000.c b/hw/e1000.c > index 80b78bc..eb323d2 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -1129,6 +1129,10 @@ static int pci_e1000_init(PCIDevice *pci_dev) > checksum = (uint16_t) EEPROM_SUM - checksum; > d->eeprom_data[EEPROM_CHECKSUM_REG] = checksum; > > +memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); > +memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init); > +d->rxbuf_min_shift = 1; > + > d->nic = qemu_new_nic(&net_e1000_info, &d->conf, >d->dev.qdev.info->name, d->dev.qdev.id, d); > >
[Qemu-devel] [PATCH] e1000: Fix hotplug
When we removed the call to e1000_reset() back in cset c1699988, we left some register state uninitialized. When we hotplug the device, we don't go through a reset cycle, which means a hot added e1000 is useless until the VM reboots. Duplicate the bits we need from e1000_reset(). Signed-off-by: Alex Williamson --- 0.13 candidate? hw/e1000.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index 80b78bc..eb323d2 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -1129,6 +1129,10 @@ static int pci_e1000_init(PCIDevice *pci_dev) checksum = (uint16_t) EEPROM_SUM - checksum; d->eeprom_data[EEPROM_CHECKSUM_REG] = checksum; +memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); +memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init); +d->rxbuf_min_shift = 1; + d->nic = qemu_new_nic(&net_e1000_info, &d->conf, d->dev.qdev.info->name, d->dev.qdev.id, d);
[Qemu-devel] Re: [Autotest][RFC PATCH 00/14] Patchset of network related subtests
On Tue, 2010-07-20 at 09:34 +0800, Amos Kong wrote: > The following series contain 11 network related subtests, welcome to give me > some suggestions about correctness, design, enhancement. > > Thank you so much! Ok Amos, now that I made the first review of this patchset, I'll wait for a v2 and supersed v1. Thanks! > --- > > Amos Kong (14): > KVM-test: Add a new macaddress pool algorithm > KVM Test: Add a function get_interface_name() to kvm_net_utils.py > KVM Test: Add a common ping module for network related tests > KVM-test: Add a new subtest ping > KVM-test: Add a subtest jumbo > KVM-test: Add basic file transfer test > KVM-test: Add a subtest of load/unload nic driver > KVM-test: Add a subtest of nic promisc > KVM-test: Add a subtest of multicast > KVM-test: Add a subtest of pxe > KVM-test: Add a subtest of changing mac address > KVM-test: Add a subtest of netperf > KVM-test: Improve vlan subtest > KVM-test: Add subtest of testing offload by ethtool > > > 0 files changed, 0 insertions(+), 0 deletions(-) >
[Qemu-devel] Re: [PATCH] [sparc32] fix last cpu timer initialization
Thanks, applied. Please remember to use Signed-off-by tag.
On Mon, Aug 2, 2010 at 5:58 PM, Artyom Tarasenko
wrote:
> The timer #0 is the system timer, so the timer #num_cpu is the
> timer of the last CPU, and it must be initialized in slavio_timer_reset.
>
> Don't mark non-existing timers as running.
> ---
> hw/slavio_timer.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/slavio_timer.c b/hw/slavio_timer.c
> index d787553..c125de4 100644
> --- a/hw/slavio_timer.c
> +++ b/hw/slavio_timer.c
> @@ -377,12 +377,12 @@ static void slavio_timer_reset(DeviceState *d)
> curr_timer->limit = 0;
> curr_timer->count = 0;
> curr_timer->reached = 0;
> - if (i < s->num_cpus) {
> + if (i <= s->num_cpus) {
> ptimer_set_limit(curr_timer->timer,
> LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
> ptimer_run(curr_timer->timer, 0);
> + curr_timer->running = 1;
> }
> - curr_timer->running = 1;
> }
> s->cputimer_mode = 0;
> }
> --
> 1.6.2.5
>
>
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On Mon, 2 Aug 2010, Christian Brunner wrote:
> After the release of ceph 0.21 I felt it was about time to re-submit
> the qemu block driver for ceph:
>
> This is an updated block driver for the distributed file system Ceph
> (http://ceph.newdream.net/). This driver uses librados (which
> is part of the Ceph server) for direct access to the Ceph object
> store and is running entirely in userspace.
>
> It now has (read only) snapshot support and passes all relevant
> qemu-iotests.
>
> To compile the driver you need at least ceph 0.21.
>
> Additional information is available on the Ceph-Wiki:
>
> http://ceph.newdream.net/wiki/Kvm-rbd
>
> The patch is based on git://repo.or.cz/qemu/kevin.git block
Thare are whitespace issues in this patch.
>
> Signed-off-by: Christian Brunner
>
> ---
> Makefile.objs |1 +
> block/rbd.c | 907
> +
> block/rbd_types.h | 71 +
> configure | 31 ++
> 4 files changed, 1010 insertions(+), 0 deletions(-)
> create mode 100644 block/rbd.c
> create mode 100644 block/rbd_types.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 4a1eaa1..bf45142 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -18,6 +18,7 @@ block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o
> block-nested-$(CONFIG_WIN32) += raw-win32.o
> block-nested-$(CONFIG_POSIX) += raw-posix.o
> block-nested-$(CONFIG_CURL) += curl.o
> +block-nested-$(CONFIG_RBD) += rbd.o
>
> block-obj-y += $(addprefix block/, $(block-nested-y))
>
> diff --git a/block/rbd.c b/block/rbd.c
> new file mode 100644
> index 000..6f8ff15
> --- /dev/null
> +++ b/block/rbd.c
> @@ -0,0 +1,907 @@
> +/*
> + * QEMU Block driver for RADOS (Ceph)
> + *
> + * Copyright (C) 2010 Christian Brunner
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "qemu-error.h"
> +#include
> +#include
> +
> +#include
> +
> +#include "rbd_types.h"
> +#include "module.h"
> +#include "block_int.h"
> +
> +#include
> +#include
> +#include
> +
> +#include
> +
> +
> +int eventfd(unsigned int initval, int flags);
> +
> +
> +/*
> + * When specifying the image filename use:
> + *
> + * rbd:poolname/devicename
> + *
> + * poolname must be the name of an existing rados pool
> + *
> + * devicename is the basename for all objects used to
> + * emulate the raw device.
> + *
> + * Metadata information (image size, ...) is stored in an
> + * object with the name "devicename.rbd".
> + *
> + * The raw device is split into 4MB sized objects by default.
> + * The sequencenumber is encoded in a 12 byte long hex-string,
> + * and is attached to the devicename, separated by a dot.
> + * e.g. "devicename.1234567890ab"
> + *
> + */
> +
> +#define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER)
> +
> +typedef struct RBDAIOCB {
> +BlockDriverAIOCB common;
> +QEMUBH *bh;
> +int ret;
> +QEMUIOVector *qiov;
> +char *bounce;
> +int write;
> +int64_t sector_num;
> +int aiocnt;
> +int error;
> +struct BDRVRBDState *s;
> +} RBDAIOCB;
> +
> +typedef struct RADOSCB {
> +int rcbid;
> +RBDAIOCB *acb;
> +int done;
> +int64_t segsize;
> +char *buf;
> +} RADOSCB;
> +
> +typedef struct BDRVRBDState {
> +int efd;
> +rados_pool_t pool;
> +rados_pool_t header_pool;
> +char name[RBD_MAX_OBJ_NAME_SIZE];
> +char block_name[RBD_MAX_BLOCK_NAME_SIZE];
> +uint64_t size;
> +uint64_t objsize;
> +int qemu_aio_count;
> +int read_only;
> +} BDRVRBDState;
> +
> +typedef struct rbd_obj_header_ondisk RbdHeader1;
> +
> +static int rbd_parsename(const char *filename, char *pool, char **snap,
> + char *name)
> +{
> +const char *rbdname;
> +char *p;
> +int l;
> +
> +if (!strstart(filename, "rbd:", &rbdname)) {
> +return -EINVAL;
> +}
> +
> +pstrcpy(pool, 2 * RBD_MAX_SEG_NAME_SIZE, rbdname);
> +p = strchr(pool, '/');
> +if (p == NULL) {
> +return -EINVAL;
> +}
> +
> +*p = '\0';
> +
> +l = strlen(pool);
> +if(l >= RBD_MAX_SEG_NAME_SIZE) {
> +error_report("pool name to long");
> +return -EINVAL;
> +} else if (l <= 0) {
> +error_report("pool name to short");
> +return -EINVAL;
> +}
> +
> +l = strlen(++p);
> +if (l >= RBD_MAX_OBJ_NAME_SIZE) {
> +error_report("object name to long");
> +return -EINVAL;
> +} else if (l <= 0) {
> +error_report("object name to short");
> +return -EINVAL;
> +}
> +
> +strcpy(name, p);
> +
> +*snap = strchr(name, '@');
> +if (*snap) {
Inconsistent indentation
> + *(*snap) = '\0';
> + (*snap)++;
> + if (!*snap) *snap = NULL;
> +}
> +
> +return l;
> +}
> +
> +static int create_tmap_op(uint8_t op, const char *name, char **tmap_desc)
> +{
> +uint32_t len
Re: [Qemu-devel] [PATCH] loader: pad kernel size when loaded from a uImage
On Mon, Aug 2, 2010 at 12:56 PM, Edgar E. Iglesias wrote: > On Mon, Aug 02, 2010 at 12:33:54PM -0700, Hollis Blanchard wrote: >> >> You mean the one architecture, which by the way doesn't even use this >> API? That doesn't seem like a strong argument to me. Anyways, it's > > Are we looking at the same code? I don't know. > Grep for load_uimage in qemu. 4 archs use it, PPC, ARM, m68k and MB. I see the following: 1 75 hw/an5206.c <> kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL); 2233 hw/arm_boot.c <> kernel_size = load_uimage(info->kernel_filename, &entry, NULL, 3 50 hw/dummy_m68k.c <> kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL); 4 14 hw/loader.h <> int load_uimage(const char *filename, target_phys_addr_t *ep, 5277 hw/mcf5208.c <> kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL); 6121 hw/ppc440_bamboo.c <> kernel_size = load_uimage(kernel...ename, &entry, &loadaddr, NULL); 7235 hw/ppce500_mpc8544ds.c <> kernel_size = load_uimage(kernel...ename, &entry, &loadaddr, NULL); That makes 2x ColdFire, ARM, M68K, 2x PowerPC. hw/petalogix_s3adsp1800_mmu.c is the only MicroBlaze I can see, and it only loads ELF and binary kernels, not uImages. > Of those archs, only 2 actually use the return value of load_uimage > to decide where to place blobs. PPC and MB. MB doesn't want any > magic applied to the return value. That leaves us with _ONE_ single > arch that needs magic that IMO is broken. You are trying to guess the > size of the loaded image's .bss area by adding a 16th of the uimage size? Accounting for BSS hardly counts as "magic", I think. :) >> just as much work to relocate an initrd from a padded address as it is >> from a closer address, so there is no downside. >> >> The fact remains that the current API is broken by design, or to be >> charitable "violates the principle of least surprise." With the >> exception of microblaze, everybody who calls load_uimage() must >> understand the nuances of the return value and adjust it (or ignore >> it) accordingly. Why wouldn't we consolidate that logic? > > I don't see how guessing that the .bss area is a 16th of the loaded > image makes this call any less confusing. I agree it's arbitrary, but it's only arbitrary in one place. It's also well-commented (IMHO), which is more than can be said for the current code. >> >> tell me: where should I hardcode initrd loading? >> > >> > Not sure, but I'd guess somewhere close to where you are calling >> > load_uimage from (it wasn't clear to me where that was). >> >> Sorry, let me rephrase. At what address should I hardcode my initrd? >> What about my device tree? As a followup, why not lower, or higher? > > You should be putting them at the same addresses as uboot puts them. Fine. It's arbitrary in u-boot too, but at least it will be consistent. -Hollis
Re: [gPXE-devel] [Qemu-devel] Netboot happens twice (first fail) when using etherboot ROMs
On 8/2/10 2:14 PM, Stefan Hajnoczi wrote: > On Mon, Aug 2, 2010 at 5:43 PM, Michael Tokarev wrote: >> Not that it is a big issue, just... weird, and annoying -- >> in Debian for example we (re)build boot ROMs during >> package build instead of using the ones supplied in >> the source tarball, and currently gpxe isn't packages >> in debian, but etherboot behaves somewhat erratically >> (but works in the end), so I wondered what the issue is. > > The issue holding back gPXE from Debian is licensing clarification > AFAIK. There is nothing there that can't be tackled but the process > has stalled unfortunately. It would be great to have gPXE in Debian. > > Stefan We definitely have not forgotten about Debian licensing issues, and in a few weeks after Google Summer of Code is over we'll go through http://support.etherboot.org/ and deal with licensing as well as some other issues we haven't gotten to yet. Thanks for reminding us about this. / Marty /
Re: [Qemu-devel] [PATCH] loader: pad kernel size when loaded from a uImage
On Mon, Aug 02, 2010 at 12:33:54PM -0700, Hollis Blanchard wrote: > On Mon, Aug 2, 2010 at 11:57 AM, Edgar E. Iglesias > wrote: > > On Mon, Aug 02, 2010 at 10:59:11AM -0700, Hollis Blanchard wrote: > >> On Sun, Aug 1, 2010 at 5:36 AM, Edgar E. Iglesias > >> wrote: > >> > On Sat, Jul 31, 2010 at 12:56:42AM +0200, Edgar E. Iglesias wrote: > >> >> On Thu, Jul 29, 2010 at 06:48:24PM -0700, Hollis Blanchard wrote: > >> >> > The kernel's BSS size is lost by mkimage, which only considers file > >> >> > size. As a result, loading other blobs (e.g. device tree, initrd) > >> >> > immediately after the kernel location can result in them being zeroed > >> >> > by > >> >> > the kernel's BSS initialization code. > >> >> > > >> >> > Signed-off-by: Hollis Blanchard > >> >> > --- > >> >> > hw/loader.c | 7 +++ > >> >> > 1 files changed, 7 insertions(+), 0 deletions(-) > >> >> > > >> >> > diff --git a/hw/loader.c b/hw/loader.c > >> >> > index 79a6f95..35bc25a 100644 > >> >> > --- a/hw/loader.c > >> >> > +++ b/hw/loader.c > >> >> > @@ -507,6 +507,13 @@ int load_uimage(const char *filename, > >> >> > target_phys_addr_t *ep, > >> >> > > >> >> > ret = hdr->ih_size; > >> >> > > >> >> > + /* The kernel's BSS size is lost by mkimage, which only considers > >> >> > file > >> >> > + * size. We don't know how big it is, but we do know we can't > >> >> > place > >> >> > + * anything immediately after the kernel. The padding seems like > >> >> > it should > >> >> > + * be proportional to overall file size, but we also make sure > >> >> > it's at > >> >> > + * least 4-byte aligned. */ > >> >> > + ret += (hdr->ih_size / 16) & ~0x3; > >> >> > >> >> Maybe it's only me, but it feels a bit akward to push down this kind of > >> >> knowledge down the abstraction layers. Does it work for you to have your > >> >> caller of load_uimage apply whatever resizing magic needed for your > >> >> kernel > >> >> and arch? > >> > > >> > Ayway, IMO the conventions of where to pass blobs from the bootloader to > >> > the > >> > loaded image are an agreement between the bootloader and the loaded > >> > code. The > >> > formats or mechanisms to load the image should need to be involved that > >> > much. > >> > > >> > For example in this particular case, other archs (e.g, MicroBlaze) might > >> > not > >> > need any magic. The MicroBlaze linux kernel moves cmdline and device > >> > tree blobs > >> > into safe areas prior to .bss initialization. > >> > >> Are you claiming that's the common case? FWIW, PowerPC and ARM don't > >> seem to. I wouldn't expect such logic except in reaction to a specific > >> bug (i.e. a qemu/firmware loader bug). > > > > I'm not trying to claim it's the common case, but it exists. > > It exists and will remain unaffected by this patch, while the common > case will be improved. > > >> The load_uimage() interface claims to report the size of the kernel it > >> loaded. If you argue that it shouldn't try to do that (and indeed you > > > > The way I understand it, it reports the size of what got loaded. > > The difference between "what got loaded" and "the size of the loaded > file in memory" is a subtlety that is not at all clear from the code, > and that is precisely why I propose centralizing the logic to handle > it. > > > It would be very difficult for load_uimage to figure out what memory > > areas are beeing touched prior to .bss init (or the point where the passed > > blob is used). > > > >> could argue it's not *possible* to do that accurately), that logic > > > > Right, its very hard for it to guess what memory areas are safe. > > > >> should be completely removed. The current behavior is worse than not > >> knowing at all: callers *think* they know, but it's guaranteed to be > >> wrong. > >> > >> Of course, if you do want to remove the size, then callers are left > >> with even less information than they had before. In that case, you > > > > I think returning the size of the loaded image has a value, for example > > for archs that move away the blobs before touching any memory outside > > their image. Bootloaders for those archs can put some blobs right after > > the loaded image. > > You mean the one architecture, which by the way doesn't even use this > API? That doesn't seem like a strong argument to me. Anyways, it's Are we looking at the same code? Grep for load_uimage in qemu. 4 archs use it, PPC, ARM, m68k and MB. Of those archs, only 2 actually use the return value of load_uimage to decide where to place blobs. PPC and MB. MB doesn't want any magic applied to the return value. That leaves us with _ONE_ single arch that needs magic that IMO is broken. You are trying to guess the size of the loaded image's .bss area by adding a 16th of the uimage size? > just as much work to relocate an initrd from a padded address as it is > from a closer address, so there is no downside. > > The fact remains that the current API is broken by design, or to be > charitable "violates the
[Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
After the release of ceph 0.21 I felt it was about time to re-submit
the qemu block driver for ceph:
This is an updated block driver for the distributed file system Ceph
(http://ceph.newdream.net/). This driver uses librados (which
is part of the Ceph server) for direct access to the Ceph object
store and is running entirely in userspace.
It now has (read only) snapshot support and passes all relevant
qemu-iotests.
To compile the driver you need at least ceph 0.21.
Additional information is available on the Ceph-Wiki:
http://ceph.newdream.net/wiki/Kvm-rbd
The patch is based on git://repo.or.cz/qemu/kevin.git block
Signed-off-by: Christian Brunner
---
Makefile.objs |1 +
block/rbd.c | 907 +
block/rbd_types.h | 71 +
configure | 31 ++
4 files changed, 1010 insertions(+), 0 deletions(-)
create mode 100644 block/rbd.c
create mode 100644 block/rbd_types.h
diff --git a/Makefile.objs b/Makefile.objs
index 4a1eaa1..bf45142 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -18,6 +18,7 @@ block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o
block-nested-$(CONFIG_WIN32) += raw-win32.o
block-nested-$(CONFIG_POSIX) += raw-posix.o
block-nested-$(CONFIG_CURL) += curl.o
+block-nested-$(CONFIG_RBD) += rbd.o
block-obj-y += $(addprefix block/, $(block-nested-y))
diff --git a/block/rbd.c b/block/rbd.c
new file mode 100644
index 000..6f8ff15
--- /dev/null
+++ b/block/rbd.c
@@ -0,0 +1,907 @@
+/*
+ * QEMU Block driver for RADOS (Ceph)
+ *
+ * Copyright (C) 2010 Christian Brunner
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "qemu-error.h"
+#include
+#include
+
+#include
+
+#include "rbd_types.h"
+#include "module.h"
+#include "block_int.h"
+
+#include
+#include
+#include
+
+#include
+
+
+int eventfd(unsigned int initval, int flags);
+
+
+/*
+ * When specifying the image filename use:
+ *
+ * rbd:poolname/devicename
+ *
+ * poolname must be the name of an existing rados pool
+ *
+ * devicename is the basename for all objects used to
+ * emulate the raw device.
+ *
+ * Metadata information (image size, ...) is stored in an
+ * object with the name "devicename.rbd".
+ *
+ * The raw device is split into 4MB sized objects by default.
+ * The sequencenumber is encoded in a 12 byte long hex-string,
+ * and is attached to the devicename, separated by a dot.
+ * e.g. "devicename.1234567890ab"
+ *
+ */
+
+#define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER)
+
+typedef struct RBDAIOCB {
+BlockDriverAIOCB common;
+QEMUBH *bh;
+int ret;
+QEMUIOVector *qiov;
+char *bounce;
+int write;
+int64_t sector_num;
+int aiocnt;
+int error;
+struct BDRVRBDState *s;
+} RBDAIOCB;
+
+typedef struct RADOSCB {
+int rcbid;
+RBDAIOCB *acb;
+int done;
+int64_t segsize;
+char *buf;
+} RADOSCB;
+
+typedef struct BDRVRBDState {
+int efd;
+rados_pool_t pool;
+rados_pool_t header_pool;
+char name[RBD_MAX_OBJ_NAME_SIZE];
+char block_name[RBD_MAX_BLOCK_NAME_SIZE];
+uint64_t size;
+uint64_t objsize;
+int qemu_aio_count;
+int read_only;
+} BDRVRBDState;
+
+typedef struct rbd_obj_header_ondisk RbdHeader1;
+
+static int rbd_parsename(const char *filename, char *pool, char **snap,
+ char *name)
+{
+const char *rbdname;
+char *p;
+int l;
+
+if (!strstart(filename, "rbd:", &rbdname)) {
+return -EINVAL;
+}
+
+pstrcpy(pool, 2 * RBD_MAX_SEG_NAME_SIZE, rbdname);
+p = strchr(pool, '/');
+if (p == NULL) {
+return -EINVAL;
+}
+
+*p = '\0';
+
+l = strlen(pool);
+if(l >= RBD_MAX_SEG_NAME_SIZE) {
+error_report("pool name to long");
+return -EINVAL;
+} else if (l <= 0) {
+error_report("pool name to short");
+return -EINVAL;
+}
+
+l = strlen(++p);
+if (l >= RBD_MAX_OBJ_NAME_SIZE) {
+error_report("object name to long");
+return -EINVAL;
+} else if (l <= 0) {
+error_report("object name to short");
+return -EINVAL;
+}
+
+strcpy(name, p);
+
+*snap = strchr(name, '@');
+if (*snap) {
+ *(*snap) = '\0';
+ (*snap)++;
+ if (!*snap) *snap = NULL;
+}
+
+return l;
+}
+
+static int create_tmap_op(uint8_t op, const char *name, char **tmap_desc)
+{
+uint32_t len = strlen(name);
+/* total_len = encoding op + name + empty buffer */
+uint32_t total_len = 1 + (sizeof(uint32_t) + len) + sizeof(uint32_t);
+char *desc = NULL;
+
+desc = qemu_malloc(total_len);
+
+*tmap_desc = desc;
+
+*desc = op;
+desc++;
+memcpy(desc, &len, sizeof(len));
+desc += sizeof(len);
+memcpy(desc, name, len);
+desc += len;
+len = 0;
+memcpy(desc, &len, sizeof(len));
+desc += sizeof(len);
+
+return desc - *tmap_desc;
+}
+
+
Re: [Qemu-devel] [PATCH] qemu-option: Include name of invalid parameter in error message
Am 02.08.2010 10:40, schrieb Markus Armbruster:
Stefan Weil writes:
All other error messages in qemu-option.c display the name
of the invalid parameter. This seems to be reasonable for
invalid identifiers, too. Without it, a debugger is needed
to find the name.
Cc: Markus Armbruster
Cc: Anthony Liguori
Signed-off-by: Stefan Weil
---
qemu-option.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 1f8f41a..ccea267 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -694,7 +694,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list,
const char *id, int fail_if_exist
if (id) {
if (!id_wellformed(id)) {
- qerror_report(QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
+ qerror_report(QERR_INVALID_PARAMETER_VALUE, id, "an identifier");
error_printf_unless_qmp("Identifiers consist of letters, digits, '-',
'.', '_', starting with a letter.\n");
return NULL;
}
No.
QERR_INVALID_PARAMETER_VALUE's first argument is the parameter *name*,
not the parameter *value*.
In this case, the parameter name is "id". The variable id contains the
parameter value.
If you need a debugger to find the offending id=, then location
information is lacking. Could you give an example where it's hard to
find the offending parameter?
Here's an example where it's easy to find:
$ qemu-system-x86_64 -device e1000,id=.
qemu-system-x86_64: -device e1000,id=.: Parameter 'id' expects an
identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a
letter.
I had a call of qemu_chr_open with an id containing a space
(which looks better in the console display where the id is shown)
in my ar7 emulation. You can test this scenario using this patch:
--- a/vl.c
+++ b/vl.c
@@ -1700,7 +1700,7 @@ static int serial_parse(const char *devname)
fprintf(stderr, "qemu: too many serial ports\n");
exit(1);
}
-snprintf(label, sizeof(label), "serial%d", index);
+snprintf(label, sizeof(label), "serial %d", index);
serial_hds[index] = qemu_chr_open(label, devname, NULL);
if (!serial_hds[index]) {
fprintf(stderr, "qemu: could not open serial device '%s': %s\n",
It results in these error messages:
qemu: Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a
letter.
qemu: could not open serial device 'vc:80Cx24C': No such file or directory
The "location information" is indeed missing. For this kind of error,
an assertion would be better than an error message.
[Qemu-devel] Re: [PATCH] PPC4xx: don't unregister RAM at reset
On 02.08.2010, at 21:37, Hollis Blanchard wrote: > On Mon, Aug 2, 2010 at 1:41 AM, Alexander Graf wrote: >> >> On 30.07.2010, at 03:48, Hollis Blanchard wrote: >> >>> The PowerPC 4xx SDRAM controller emulation unregisters RAM in its reset >>> callback. However, qemu_system_reset() is now called at initialization >>> time, so RAM is unregistered before starting the guest. >> >> So the registration should be moved to reset now, no? How is the reset >> different from boot? How did a reset work before? > > As far as I can tell, no other platform unregisters and re-registers > memory at reset, so that is a difference between reset and boot. > > Maybe I don't understand your other question. Before > qemu_system_reset() was called at initialization time, memory was not > unregistered, and therefore the platform had memory and could boot. Then removal of the unregister is sane and the reset path was broken before. That's good to know :). Alex
[Qemu-devel] Re: [PATCH] PPC4xx: don't unregister RAM at reset
On Mon, Aug 2, 2010 at 1:41 AM, Alexander Graf wrote: > > On 30.07.2010, at 03:48, Hollis Blanchard wrote: > >> The PowerPC 4xx SDRAM controller emulation unregisters RAM in its reset >> callback. However, qemu_system_reset() is now called at initialization >> time, so RAM is unregistered before starting the guest. > > So the registration should be moved to reset now, no? How is the reset > different from boot? How did a reset work before? As far as I can tell, no other platform unregisters and re-registers memory at reset, so that is a difference between reset and boot. Maybe I don't understand your other question. Before qemu_system_reset() was called at initialization time, memory was not unregistered, and therefore the platform had memory and could boot. -Hollis
Re: [Qemu-devel] [PATCH] loader: pad kernel size when loaded from a uImage
On Mon, Aug 2, 2010 at 11:57 AM, Edgar E. Iglesias wrote: > On Mon, Aug 02, 2010 at 10:59:11AM -0700, Hollis Blanchard wrote: >> On Sun, Aug 1, 2010 at 5:36 AM, Edgar E. Iglesias >> wrote: >> > On Sat, Jul 31, 2010 at 12:56:42AM +0200, Edgar E. Iglesias wrote: >> >> On Thu, Jul 29, 2010 at 06:48:24PM -0700, Hollis Blanchard wrote: >> >> > The kernel's BSS size is lost by mkimage, which only considers file >> >> > size. As a result, loading other blobs (e.g. device tree, initrd) >> >> > immediately after the kernel location can result in them being zeroed by >> >> > the kernel's BSS initialization code. >> >> > >> >> > Signed-off-by: Hollis Blanchard >> >> > --- >> >> > hw/loader.c | 7 +++ >> >> > 1 files changed, 7 insertions(+), 0 deletions(-) >> >> > >> >> > diff --git a/hw/loader.c b/hw/loader.c >> >> > index 79a6f95..35bc25a 100644 >> >> > --- a/hw/loader.c >> >> > +++ b/hw/loader.c >> >> > @@ -507,6 +507,13 @@ int load_uimage(const char *filename, >> >> > target_phys_addr_t *ep, >> >> > >> >> > ret = hdr->ih_size; >> >> > >> >> > + /* The kernel's BSS size is lost by mkimage, which only considers >> >> > file >> >> > + * size. We don't know how big it is, but we do know we can't place >> >> > + * anything immediately after the kernel. The padding seems like it >> >> > should >> >> > + * be proportional to overall file size, but we also make sure it's >> >> > at >> >> > + * least 4-byte aligned. */ >> >> > + ret += (hdr->ih_size / 16) & ~0x3; >> >> >> >> Maybe it's only me, but it feels a bit akward to push down this kind of >> >> knowledge down the abstraction layers. Does it work for you to have your >> >> caller of load_uimage apply whatever resizing magic needed for your kernel >> >> and arch? >> > >> > Ayway, IMO the conventions of where to pass blobs from the bootloader to >> > the >> > loaded image are an agreement between the bootloader and the loaded code. >> > The >> > formats or mechanisms to load the image should need to be involved that >> > much. >> > >> > For example in this particular case, other archs (e.g, MicroBlaze) might >> > not >> > need any magic. The MicroBlaze linux kernel moves cmdline and device tree >> > blobs >> > into safe areas prior to .bss initialization. >> >> Are you claiming that's the common case? FWIW, PowerPC and ARM don't >> seem to. I wouldn't expect such logic except in reaction to a specific >> bug (i.e. a qemu/firmware loader bug). > > I'm not trying to claim it's the common case, but it exists. It exists and will remain unaffected by this patch, while the common case will be improved. >> The load_uimage() interface claims to report the size of the kernel it >> loaded. If you argue that it shouldn't try to do that (and indeed you > > The way I understand it, it reports the size of what got loaded. The difference between "what got loaded" and "the size of the loaded file in memory" is a subtlety that is not at all clear from the code, and that is precisely why I propose centralizing the logic to handle it. > It would be very difficult for load_uimage to figure out what memory > areas are beeing touched prior to .bss init (or the point where the passed > blob is used). > >> could argue it's not *possible* to do that accurately), that logic > > Right, its very hard for it to guess what memory areas are safe. > >> should be completely removed. The current behavior is worse than not >> knowing at all: callers *think* they know, but it's guaranteed to be >> wrong. >> >> Of course, if you do want to remove the size, then callers are left >> with even less information than they had before. In that case, you > > I think returning the size of the loaded image has a value, for example > for archs that move away the blobs before touching any memory outside > their image. Bootloaders for those archs can put some blobs right after > the loaded image. You mean the one architecture, which by the way doesn't even use this API? That doesn't seem like a strong argument to me. Anyways, it's just as much work to relocate an initrd from a padded address as it is from a closer address, so there is no downside. The fact remains that the current API is broken by design, or to be charitable "violates the principle of least surprise." With the exception of microblaze, everybody who calls load_uimage() must understand the nuances of the return value and adjust it (or ignore it) accordingly. Why wouldn't we consolidate that logic? >> tell me: where should I hardcode initrd loading? > > Not sure, but I'd guess somewhere close to where you are calling > load_uimage from (it wasn't clear to me where that was). Sorry, let me rephrase. At what address should I hardcode my initrd? What about my device tree? As a followup, why not lower, or higher? Also, how can I know in the code if I chose wrong, what will the user-visible failure be, and how difficult will that be to debug? In summary, this patch protects users and developers. If I move it
[Qemu-devel] Re: [RFC PATCH 14/14] KVM-test: Add subtest of testing offload by ethtool
On Tue, 2010-07-20 at 09:36 +0800, Amos Kong wrote:
> The latest case contains TX/RX/SG/TSO/GSO/GRO/LRO test. RTL8139 NIC doesn't
> support TSO, LRO, it's too old, so drop offload test from rtl8139. LRO, GRO
> are only supported by latest kernel, virtio nic doesn't support receive
> offloading function.
> Initialize the callbacks first and execute all the sub tests one by one, all
> the result will be check at the end.
> When execute this test, vhost should be enabled, then most of new feature can
> be used. Vhost doestn't support VIRTIO_NET_F_MRG_RXBUF, so do not check large
> packets in received offload test.
> Transfer files by scp between host and guest, match new opened TCP port by
> netstat. Capture the packages info by tcpdump, it contains package length.
This test is heavily dependent on ethtool, so we need to make sure the
package is going to be installed on linux guests. The default package
selection for Fedora 13 does not include it for example. So, we need to
modify linux guest kickstarts/XMLs to add ethtool to the default package
selection.
> Signed-off-by: Amos Kong
> ---
> 0 files changed, 0 insertions(+), 0 deletions(-)
>
> diff --git a/client/tests/kvm/tests/ethtool.py
> b/client/tests/kvm/tests/ethtool.py
> new file mode 100644
> index 000..7274eae
> --- /dev/null
> +++ b/client/tests/kvm/tests/ethtool.py
> @@ -0,0 +1,205 @@
> +import time, os, logging, commands, re
> +from autotest_lib.client.common_lib import error
> +from autotest_lib.client.bin import utils
> +import kvm_test_utils, kvm_utils, kvm_net_utils
> +
> +def run_ethtool(test, params, env):
> +"""
> +Test offload functions of ethernet device by ethtool
> +
> +1) Log into a guest
> +2) Initialize the callback of sub functions
> +3) Enable/disable sub function of NIC
> +4) Execute callback function
> +5) Check the return value
> +6) Restore original configuration
> +
> +@param test: Kvm test object
> +@param params: Dictionary with the test parameters.
> +@param env: Dictionary with test environment.
> +"""
> +def ethtool_get(type):
> +feature_pattern = {
> +'tx': 'tx.*checksumming',
> +'rx': 'rx.*checksumming',
> +'sg': 'scatter.*gather',
> +'tso': 'tcp.*segmentation.*offload',
> +'gso': 'generic.*segmentation.*offload',
> +'gro': 'generic.*receive.*offload',
> +'lro': 'large.*receive.*offload',
> +}
> +s, o = session.get_command_status_output("ethtool -k %s" % ethname)
> +try:
> +return re.findall("%s: (.*)" % feature_pattern.get(type), o)[0]
> +except IndexError:
> +logging.debug("Could not get %s status" % type)
> +
> +def ethtool_set(type, status):
> +"""
> +Set ethernet device offload status
> +
> +@param type: Offload type name
> +@param status: New status will be changed to
> +"""
> +logging.info("Try to set %s %s" % (type, status))
> +if status not in ["off", "on"]:
> +return False
> +cmd = "ethtool -K %s %s %s" % (ethname, type, status)
> +if ethtool_get(type) != status:
> +return session.get_command_status(cmd) == 0
> +if ethtool_get(type) != status:
> +logging.error("Fail to set %s %s" % (type, status))
> +return False
> +return True
> +
> +def ethtool_save_params():
> +logging.info("Save ethtool configuration")
> +for i in supported_features:
> +feature_status[i] = ethtool_get(i)
> +
> +def ethtool_restore_params():
> +logging.info("Restore ethtool configuration")
> +for i in supported_features:
> +ethtool_set(i, feature_status[i])
> +
> +def compare_md5sum(name):
> +logging.info("Compare md5sum of the files on guest and host")
> +host_result = utils.hash_file(name, method="md5")
> +try:
> +o = session.get_command_output("md5sum %s" % name)
> +guest_result = re.findall("\w+", o)[0]
> +except IndexError:
> +logging.error("Could not get file md5sum in guest")
> +return False
> +logging.debug("md5sum: guest(%s), host(%s)" % (guest_result,
> + host_result))
> +return guest_result == host_result
> +
> +def transfer_file(src="guest"):
> +"""
> +Transfer file by scp, use tcpdump to capture packets, then check the
> +return string.
> +
> +@param src: Source host of transfer file
> +@return: Tuple (status, error msg/tcpdump result)
> +"""
> +session2.get_command_status("rm -rf %s" % filename)
> +dd_cmd = "dd if=/dev/urandom of=%s bs=1M count=%s" % (filename,
> + params.get("filesize"))
> +logging.info("Creat file in source host, cmd: %s" %
Re: [Qemu-devel] [PATCH] loader: pad kernel size when loaded from a uImage
On Mon, Aug 02, 2010 at 10:59:11AM -0700, Hollis Blanchard wrote: > On Sun, Aug 1, 2010 at 5:36 AM, Edgar E. Iglesias > wrote: > > On Sat, Jul 31, 2010 at 12:56:42AM +0200, Edgar E. Iglesias wrote: > >> On Thu, Jul 29, 2010 at 06:48:24PM -0700, Hollis Blanchard wrote: > >> > The kernel's BSS size is lost by mkimage, which only considers file > >> > size. As a result, loading other blobs (e.g. device tree, initrd) > >> > immediately after the kernel location can result in them being zeroed by > >> > the kernel's BSS initialization code. > >> > > >> > Signed-off-by: Hollis Blanchard > >> > --- > >> > hw/loader.c | 7 +++ > >> > 1 files changed, 7 insertions(+), 0 deletions(-) > >> > > >> > diff --git a/hw/loader.c b/hw/loader.c > >> > index 79a6f95..35bc25a 100644 > >> > --- a/hw/loader.c > >> > +++ b/hw/loader.c > >> > @@ -507,6 +507,13 @@ int load_uimage(const char *filename, > >> > target_phys_addr_t *ep, > >> > > >> > ret = hdr->ih_size; > >> > > >> > + /* The kernel's BSS size is lost by mkimage, which only considers > >> > file > >> > + * size. We don't know how big it is, but we do know we can't place > >> > + * anything immediately after the kernel. The padding seems like it > >> > should > >> > + * be proportional to overall file size, but we also make sure it's > >> > at > >> > + * least 4-byte aligned. */ > >> > + ret += (hdr->ih_size / 16) & ~0x3; > >> > >> Maybe it's only me, but it feels a bit akward to push down this kind of > >> knowledge down the abstraction layers. Does it work for you to have your > >> caller of load_uimage apply whatever resizing magic needed for your kernel > >> and arch? > > > > Ayway, IMO the conventions of where to pass blobs from the bootloader to the > > loaded image are an agreement between the bootloader and the loaded code. > > The > > formats or mechanisms to load the image should need to be involved that > > much. > > > > For example in this particular case, other archs (e.g, MicroBlaze) might not > > need any magic. The MicroBlaze linux kernel moves cmdline and device tree > > blobs > > into safe areas prior to .bss initialization. > > Are you claiming that's the common case? FWIW, PowerPC and ARM don't > seem to. I wouldn't expect such logic except in reaction to a specific > bug (i.e. a qemu/firmware loader bug). I'm not trying to claim it's the common case, but it exists. BTW, qemu-arm seems to follow a convention to place initrd 8Mb above RAM base, it doesn't look at the loaded uimage size when deciding where to place initrd. > The load_uimage() interface claims to report the size of the kernel it > loaded. If you argue that it shouldn't try to do that (and indeed you The way I understand it, it reports the size of what got loaded. It would be very difficult for load_uimage to figure out what memory areas are beeing touched prior to .bss init (or the point where the passed blob is used). > could argue it's not *possible* to do that accurately), that logic Right, its very hard for it to guess what memory areas are safe. > should be completely removed. The current behavior is worse than not > knowing at all: callers *think* they know, but it's guaranteed to be > wrong. > > Of course, if you do want to remove the size, then callers are left > with even less information than they had before. In that case, you I think returning the size of the loaded image has a value, for example for archs that move away the blobs before touching any memory outside their image. Bootloaders for those archs can put some blobs right after the loaded image. > tell me: where should I hardcode initrd loading? Not sure, but I'd guess somewhere close to where you are calling load_uimage from (it wasn't clear to me where that was). Take a look at how arm does it in hw/arm_boot.c. CRIS doesn't have uimage support now, but if it had It would probably do whatever magic was needed in it's dedicated boot loader file, hw/cris-boot.c. Microblaze has uimage support, look in hw/petalogix_s3adsp1800_mmu.c. Maybe we should consider adding a ppc-boot.c with boot-loader magics? Cheers, Edgar
Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present
On 08/02/2010 01:10 PM, Kevin O'Connor wrote: On Mon, Aug 02, 2010 at 11:52:22AM -0500, Anthony Liguori wrote: BTW, we need to document somewhere any assumptions SeaBIOS has about the JPEG. I see that it expects a 1024x768 image. Any additional restrictions on the jpeg image? I listed some notes in a previous email: Right, we need this in either docs/seabios.txt in qemu.git or in a file in seabios.git I think. Regards, Anthony Liguori Some notes: This uses the qemu "rom" interface for loading the jpeg file. It seems to work, but I'm not sure if this is strictly correct. The jpeg viewer in SeaBIOS will look at the image size and try to find a vesa graphics mode that supports that size. So, choose images that are exactly 640x480, 1024x768, etc. Also, the SeaBIOS viewer has stripped down support for jpegs - not all valid jpegs will work. Some quick tests with the netpbm tools worked okay for me. SeaBIOS only shows the bootsplash during the interval between vgabios init and OS execution. This is generally too short a time to be seen. Add "-menu boot=on" to the qemu command line to have it shown longer. Unfortunately, the vgabios doesn't support writing text to the screen while in a vesa video mode. So, this means that if a user selects F12 for the boot menu, they can't actually see the boot menu. This will need to be fixed in SeaBIOS in a follow up patch. -Kevin
[Qemu-devel] [Bug 521994] Re: Windows 98 doesn't detect mouse on qemu and SeaBIOS.
** Changed in: qemu-kvm (Debian) Status: New => Fix Committed -- Windows 98 doesn't detect mouse on qemu and SeaBIOS. https://bugs.launchpad.net/bugs/521994 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Confirmed Status in “qemu-kvm” package in Debian: Fix Committed Bug description: A windows 98 guest doesn't detect mouse on recent qemu. I bisected and the result is fd646122418ecefcde228d43821d07da79dd99bb is the first bad commit commit fd646122418ecefcde228d43821d07da79dd99bb Author: Anthony Liguori Date: Fri Oct 30 09:06:09 2009 -0500 Switch pc bios from pc-bios to seabios SeaBIOS is a port of pc-bios to GCC. Besides using a more modern tool chain, SeaBIOS introduces a number of new features including PMM support, better BEV and BCV support, and better PnP support. Signed-off-by: Anthony Liguori I got following messages with DEBUG_BIOS Start bios (version 0.5.1-20100111_132716-squirrel.codemonkey.ws) Ram Size=0x0800 (0x high) CPU Mhz=2271 Found 1 cpu(s) max supported 1 cpu(s) PIIX3/PIIX4 init: elcr=00 0c PCI: bus=0 devfn=0x00: vendor_id=0x8086 device_id=0x1237 PCI: bus=0 devfn=0x08: vendor_id=0x8086 device_id=0x7000 PCI: bus=0 devfn=0x09: vendor_id=0x8086 device_id=0x7010 region 4: 0xc000 PCI: bus=0 devfn=0x0b: vendor_id=0x8086 device_id=0x7113 PCI: bus=0 devfn=0x10: vendor_id=0x1013 device_id=0x00b8 region 0: 0xe000 region 1: 0xe200 region 6: 0xe201 MP table addr=0x000f89b0 MPC table addr=0x000f89c0 size=224 SMBIOS ptr=0x000f8990 table=0x07fffef0 ACPI tables: RSDP=0x000f8960 RSDT=0x07ffde30 Scan for VGA option rom Running option rom at c000:0003 VGABios $Id$ Turning on vga console Starting SeaBIOS (version 0.5.1-20100111_132716-squirrel.codemonkey.ws) Found 0 lpt ports Found 0 serial ports ATA controller 0 at 1f0/3f4/c000 (irq 14 dev 9) ATA controller 1 at 170/374/c008 (irq 15 dev 9) ps2 irq but no data. ata0-0: PCHS=812/16/63 translation=none LCHS=812/16/63 ata0-1: PCHS=1152/16/56 translation=none LCHS=1024/16/56 ps2_recvbyte timeout keyboard initialized Scan for option roms Returned 53248 bytes of ZoneHigh e820 map has 6 items: 0: - 0009f400 = 1 1: 0009f400 - 000a = 2 2: 000f - 0010 = 2 3: 0010 - 07ffd000 = 1 4: 07ffd000 - 0800 = 2 5: fffc - 0001 = 2 enter handle_19: NULL Booting from Hard Disk... Booting from :7c00 pnp call arg1=5 pnp call arg1=0 ps2_recvbyte timeout ps2_recvbyte timeout ps2_recvbyte timeout ps2_recvbyte timeout
[Qemu-devel] [Bug 586175] Re: Windows XP/2003 doesn't boot
Andreas, The program that created the disk image seems confused, but it worked for creating a VM for FC11. Windows install seems to run fine, until wanting to boot from the drive it created. I don't know what creates the drive image and geometry, but it is broken. I think this is what I used to create the VM, but I have messed around with so many configurations and methods, I'm not sure what is what anymore. virt-install --connect qemu:///system -n w7 -r 2048 --vcpus=2 \ --disk path=/vm/w7.img,size=20,sparse=false,format=qcow2 \ -c /vm/w7cd.iso --vnc --noautoconsole \ --os-type windows --os-variant win7 --accelerate --network=bridge:br0 --hvm How many thousands of people have struggled with this and also got nowhere? It just looks like the virt-install developers have not tasted their own dogfood! LVM is supposed to be easy - just select vm image and boot, but the more I read about VMs, kvm, qemu, virtualbox, virsh etc, the more confused I get on how they relate to each other. testdisk reports this: ~~ Disk /dev/loop0 - 21 GB / 20 GiB - CHS 41943040 1 1 (wtf ??) Partition StartEndSize in sectors 1 * HPFS - NTFS 2048 206847 204800 [System Reserved] 2 P HPFS - NTFS 206848 41940991 41734144 Select 1: Disk /dev/loop0 - 21 GB / 20 GiB - CHS 41943040 1 1 Partition StartEndSize in sectors 1 * HPFS - NTFS 2048 206847 204800 [System Reserved] Boot sector Warning: Incorrect number of heads/cylinder 16 (NTFS) != 1 (HD) Warning: Incorrect number of sectors per track 63 (NTFS) != 1 (HD) Status: OK Backup boot sector Warning: Incorrect number of heads/cylinder 16 (NTFS) != 1 (HD) Warning: Incorrect number of sectors per track 63 (NTFS) != 1 (HD) Status: OK Sectors are identical. A valid NTFS Boot sector must be present in order to access any data; even if the partition is not bootable. ~~ Rebuild BS: ~~ Disk /dev/loop0 - 21 GB / 20 GiB - CHS 41943040 1 1 Partition StartEndSize in sectors 1 * HPFS - NTFS 2048 206847 204800 [System Reserved] filesystem size 204800 204800 sectors_per_cluster 8 8 mft_lcn 8533 8533 mftmirr_lcn 2 2 clusters_per_mft_record -10 -10 clusters_per_index_record 1 1 Extrapolated boot sector and current boot sector are different. ~~ Q Select 2: ~~ Disk /dev/loop0 - 21 GB / 20 GiB - CHS 41943040 1 1 Partition StartEndSize in sectors 2 P HPFS - NTFS 206848 41940991 41734144 Boot sector Warning: Incorrect number of heads/cylinder 16 (NTFS) != 1 (HD) Warning: Incorrect number of sectors per track 63 (NTFS) != 1 (HD) Status: OK Backup boot sector Warning: Incorrect number of heads/cylinder 16 (NTFS) != 1 (HD) Warning: Incorrect number of sectors per track 63 (NTFS) != 1 (HD) Status: OK Sectors are identical. A valid NTFS Boot sector must be present in order to access any data; even if the partition is not bootable. ~~ Rebuild BS: ~~ Disk /dev/loop0 - 21 GB / 20 GiB - CHS 41943040 1 1 Partition StartEndSize in sectors 2 P HPFS - NTFS 206848 41940991 41734144 filesystem size 41734144 41734144 sectors_per_cluster 8 8 mft_lcn 786432 786432 mftmirr_lcn 2 2 clusters_per_mft_record -10 -10 clusters_per_index_record 1 1 Extrapolated boot sector and current boot sector are different. ~~ It looks a mess. -- Windows XP/2003 doesn't boot https://bugs.launchpad.net/bugs/586175 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Status in “qemu-kvm” package in Ubuntu: New Status in Debian GNU/Linux: New Status in Fedora: Unknown Bug description: Hello everyone, my qemu doesn't boot any Windows XP/2003 installations if I try to boot the image. If I boot the install cd first, it's boot manager counts down and triggers the boot on it's own. That's kinda stupid. I'm using libvirt, but even by a simple > qemu-kvm -drive file=image.img,media=disk,if=ide,boot=on it won't boot. Qemu hangs at the message "Booting from Hard Disk..." I'm using qemu-kvm-0.12.4 with SeaBIOS 0.5.1 on Gentoo (No-Multilib and AMD64). It's a server, that means I'm using VNC as the primary graphic output but i don't think it should be an issue.
Re: [Qemu-devel] Netboot happens twice (first fail) when using etherboot ROMs
On Mon, Aug 2, 2010 at 5:43 PM, Michael Tokarev wrote: > Not that it is a big issue, just... weird, and annoying -- > in Debian for example we (re)build boot ROMs during > package build instead of using the ones supplied in > the source tarball, and currently gpxe isn't packages > in debian, but etherboot behaves somewhat erratically > (but works in the end), so I wondered what the issue is. The issue holding back gPXE from Debian is licensing clarification AFAIK. There is nothing there that can't be tackled but the process has stalled unfortunately. It would be great to have gPXE in Debian. Stefan
Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present
On Mon, Aug 02, 2010 at 11:52:22AM -0500, Anthony Liguori wrote: > BTW, we need to document somewhere any assumptions SeaBIOS has about > the JPEG. I see that it expects a 1024x768 image. Any additional > restrictions on the jpeg image? I listed some notes in a previous email: >> Some notes: >> >> This uses the qemu "rom" interface for loading the jpeg file. It >> seems to work, but I'm not sure if this is strictly correct. >> >> The jpeg viewer in SeaBIOS will look at the image size and try to find >> a vesa graphics mode that supports that size. So, choose images that >> are exactly 640x480, 1024x768, etc. Also, the SeaBIOS viewer has >> stripped down support for jpegs - not all valid jpegs will work. Some >> quick tests with the netpbm tools worked okay for me. >> >> SeaBIOS only shows the bootsplash during the interval between vgabios >> init and OS execution. This is generally too short a time to be seen. >> Add "-menu boot=on" to the qemu command line to have it shown longer. >> >> Unfortunately, the vgabios doesn't support writing text to the screen >> while in a vesa video mode. So, this means that if a user selects F12 >> for the boot menu, they can't actually see the boot menu. This will >> need to be fixed in SeaBIOS in a follow up patch. -Kevin
[Qemu-devel] [Bug 612677] [NEW] qemu-kvm -curses displays garbled screen
Public bug reported: when I launch qemu-kvm -curses (even without a guest OS) I get a garbled output, here's a screenshot: http://kontsevoy.com/qemu.png some more info: myarch ~: uname -a Linux myarch 2.6.34-ARCH #1 SMP PREEMPT Mon Jul 5 22:12:11 CEST 2010 x86_64 Intel(R) Core(TM)2 Duo CPU P8700 @ 2.53GHz GenuineIntel GNU/Linux myarch ~: qemu-kvm --version QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 Fabrice Bellard I also fetched the latest qemu-kvm from git repo and compiled it with simple ./configure&make The compiled version behaved similarly I also tried different terminal emulators: gnome-terminal and xterm - same thing I also tried real terminal (i.e. booted without X) - same thing Then I thought maybe the issue is my hardware (if its possible) and booted into Ubuntu on the same machine, installed KVM for Ubuntu and it worked there. ** Affects: qemu Importance: Undecided Status: New ** Description changed: when I launch qemu-kvm -curses (even without a guest OS) I get a garbled output, here's a screenshot: http://kontsevoy.com/qemu.png some more info: myarch ~: uname -a Linux myarch 2.6.34-ARCH #1 SMP PREEMPT Mon Jul 5 22:12:11 CEST 2010 x86_64 Intel(R) Core(TM)2 Duo CPU P8700 @ 2.53GHz GenuineIntel GNU/Linux myarch ~: qemu-kvm --version QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 Fabrice Bellard I also fetched the latest qemu-kvm from git repo and compiled it with simple ./configure&make The compiled version behaved similarly I also tried different terminal emulators: gnome-terminal and xterm - same thing I also tried real terminal (i.e. booted without X) - same thing + + Then I thought maybe the issue is my hardware (if its possible) and + booted into Ubuntu on the same machine, installed KVM for Ubuntu and it + worked there. -- qemu-kvm -curses displays garbled screen https://bugs.launchpad.net/bugs/612677 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: when I launch qemu-kvm -curses (even without a guest OS) I get a garbled output, here's a screenshot: http://kontsevoy.com/qemu.png some more info: myarch ~: uname -a Linux myarch 2.6.34-ARCH #1 SMP PREEMPT Mon Jul 5 22:12:11 CEST 2010 x86_64 Intel(R) Core(TM)2 Duo CPU P8700 @ 2.53GHz GenuineIntel GNU/Linux myarch ~: qemu-kvm --version QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 Fabrice Bellard I also fetched the latest qemu-kvm from git repo and compiled it with simple ./configure&make The compiled version behaved similarly I also tried different terminal emulators: gnome-terminal and xterm - same thing I also tried real terminal (i.e. booted without X) - same thing Then I thought maybe the issue is my hardware (if its possible) and booted into Ubuntu on the same machine, installed KVM for Ubuntu and it worked there.
[Qemu-devel] [PATCH] [sparc32] fix last cpu timer initialization
The timer #0 is the system timer, so the timer #num_cpu is the
timer of the last CPU, and it must be initialized in slavio_timer_reset.
Don't mark non-existing timers as running.
---
hw/slavio_timer.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/slavio_timer.c b/hw/slavio_timer.c
index d787553..c125de4 100644
--- a/hw/slavio_timer.c
+++ b/hw/slavio_timer.c
@@ -377,12 +377,12 @@ static void slavio_timer_reset(DeviceState *d)
curr_timer->limit = 0;
curr_timer->count = 0;
curr_timer->reached = 0;
-if (i < s->num_cpus) {
+if (i <= s->num_cpus) {
ptimer_set_limit(curr_timer->timer,
LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
ptimer_run(curr_timer->timer, 0);
+curr_timer->running = 1;
}
-curr_timer->running = 1;
}
s->cputimer_mode = 0;
}
--
1.6.2.5
Re: [Qemu-devel] [PATCH] loader: pad kernel size when loaded from a uImage
On Sun, Aug 1, 2010 at 5:36 AM, Edgar E. Iglesias wrote: > On Sat, Jul 31, 2010 at 12:56:42AM +0200, Edgar E. Iglesias wrote: >> On Thu, Jul 29, 2010 at 06:48:24PM -0700, Hollis Blanchard wrote: >> > The kernel's BSS size is lost by mkimage, which only considers file >> > size. As a result, loading other blobs (e.g. device tree, initrd) >> > immediately after the kernel location can result in them being zeroed by >> > the kernel's BSS initialization code. >> > >> > Signed-off-by: Hollis Blanchard >> > --- >> > hw/loader.c | 7 +++ >> > 1 files changed, 7 insertions(+), 0 deletions(-) >> > >> > diff --git a/hw/loader.c b/hw/loader.c >> > index 79a6f95..35bc25a 100644 >> > --- a/hw/loader.c >> > +++ b/hw/loader.c >> > @@ -507,6 +507,13 @@ int load_uimage(const char *filename, >> > target_phys_addr_t *ep, >> > >> > ret = hdr->ih_size; >> > >> > + /* The kernel's BSS size is lost by mkimage, which only considers file >> > + * size. We don't know how big it is, but we do know we can't place >> > + * anything immediately after the kernel. The padding seems like it >> > should >> > + * be proportional to overall file size, but we also make sure it's at >> > + * least 4-byte aligned. */ >> > + ret += (hdr->ih_size / 16) & ~0x3; >> >> Maybe it's only me, but it feels a bit akward to push down this kind of >> knowledge down the abstraction layers. Does it work for you to have your >> caller of load_uimage apply whatever resizing magic needed for your kernel >> and arch? > > Ayway, IMO the conventions of where to pass blobs from the bootloader to the > loaded image are an agreement between the bootloader and the loaded code. The > formats or mechanisms to load the image should need to be involved that much. > > For example in this particular case, other archs (e.g, MicroBlaze) might not > need any magic. The MicroBlaze linux kernel moves cmdline and device tree > blobs > into safe areas prior to .bss initialization. Are you claiming that's the common case? FWIW, PowerPC and ARM don't seem to. I wouldn't expect such logic except in reaction to a specific bug (i.e. a qemu/firmware loader bug). The load_uimage() interface claims to report the size of the kernel it loaded. If you argue that it shouldn't try to do that (and indeed you could argue it's not *possible* to do that accurately), that logic should be completely removed. The current behavior is worse than not knowing at all: callers *think* they know, but it's guaranteed to be wrong. Of course, if you do want to remove the size, then callers are left with even less information than they had before. In that case, you tell me: where should I hardcode initrd loading? Anyways, you don't even use load_uimage() in microblaze, and if you did, you wouldn't be obligated to use the "size" return value, so fixing this issue for everybody else doesn't limit you at all. -Hollis
Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole
On 08/02/2010 12:28 PM, Alon Levy wrote:
- "Anthony Liguori" wrote:
On 08/02/2010 03:33 AM, Alon Levy wrote:
Hi,
This patch adds three CHR_IOCTLs and uses them in virtserial
devices, to be used
by a chardev backend, such as a spice vm channel (spice is a vdi
solution).
Basically virtio-serial provides three driver initiated events for
guest open of
a device, guest close, and guest ready (driver port init complete)
that before this
patch are not exposed to the chardev backend.
With the spicevmc backend this is used like this:
qemu -chardev spicevmc,id=vdiport,name=vdiport -device
virtserialport,chardev=vdiport,name=com.redhat.spice.0
I'd appreciate any feedback if this seems the right way to
accomplish this, and
for the numbers I grabbed.
I really hate to add connection semantics via IOCTLs. I would rather
we
add them as first class semantics to the char device layer. This
would
allow us to use char devices for VNC, for instance.
Ok, that's actually what I wanted to do at first, how about:
My main objection to ioctls is that you change states based on event
delivery. This results in weird things like what happens when you do a
chr_write while not ready or not connected.
So what I'd rather see is a move to an API that was connection
oriented. For instance, we could treat CharDriverState as an
established connection. So something like:
typedef struct CharServerState
{
int backlog; /* max simultaneous connections; -1 for unlimited */
void (*connect)(CharServerState *s, CharDriverState *session);
void (*disconnect)(CharServerState *s, CharDriverState *session);
} CharDriverState;
Obviously, more thought is needed but I hope the point comes across. We
should be able to reflect the connect/disconnect semantics with an
object that has a life cycle that matches the session instead of forcing
each user to keep track of the session's life cycle.
Regards,
Anthony Liguori
diff --git a/qemu-char.h b/qemu-char.h
index 1df53ae..22973cd 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -16,6 +16,8 @@
#define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
#define CHR_EVENT_CLOSED 5 /* connection closed */
+#define CHR_DEVICE_EVENT_OPENED 0
+#define CHR_DEVICE_EVENT_CLOSED 1
#define CHR_IOCTL_SERIAL_SET_PARAMS 1
typedef struct {
@@ -59,6 +61,7 @@ struct CharDriverState {
int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
void (*chr_update_read_handler)(struct CharDriverState *s);
int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
+int (*chr_device_event)(struct CharDriverState *s, int cmd, void *arg);
int (*get_msgfd)(struct CharDriverState *s);
IOEventHandler *chr_event;
IOCanReadHandler *chr_can_read;
@@ -83,6 +86,7 @@ void qemu_chr_close(CharDriverState *chr);
void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
void qemu_chr_send_event(CharDriverState *s, int event);
+void qemu_chr_send_device_event(CharDriverState *s, int event, void *arg);
void qemu_chr_add_handlers(CharDriverState *s,
IOCanReadHandler *fd_can_read,
IOReadHandler *fd_read,
Regards,
Anthony Liguori
Alon
-- commit message
From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:00
2001
From: Alon Levy
Date: Mon, 2 Aug 2010 11:22:58 +0300
Subject: [PATCH] virtio-console: add IOCTL's for
guest_{ready,open,close}
Add three IOCTL corresponding to the three control events of:
guest_ready -> CHR_IOCTL_VIRT_SERIAL_READY
guest_open -> CHR_IOCTL_VIRT_SERIAL_OPEN
guest_close -> CHR_IOCTL_VIRT_SERIAL_CLOSE
Can be used by a matching backend.
---
hw/virtio-console.c | 33 +
qemu-char.h |4
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index caea11f..4c3686d 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -58,6 +58,33 @@ static void chr_event(void *opaque, int event)
}
}
+static void virtconsole_guest_open(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_OPEN,
NULL);
+}
+}
+
+static void virtconsole_guest_close(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_CLOSE,
NULL);
+}
+}
+
+static void virtconsole_guest_ready(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr)
Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole
- "Anthony Liguori" wrote:
> On 08/02/2010 03:33 AM, Alon Levy wrote:
> > Hi,
> >
> > This patch adds three CHR_IOCTLs and uses them in virtserial
> devices, to be used
> > by a chardev backend, such as a spice vm channel (spice is a vdi
> solution).
> >
> > Basically virtio-serial provides three driver initiated events for
> guest open of
> > a device, guest close, and guest ready (driver port init complete)
> that before this
> > patch are not exposed to the chardev backend.
> >
> > With the spicevmc backend this is used like this:
> > qemu -chardev spicevmc,id=vdiport,name=vdiport -device
> virtserialport,chardev=vdiport,name=com.redhat.spice.0
> >
> > I'd appreciate any feedback if this seems the right way to
> accomplish this, and
> > for the numbers I grabbed.
> >
>
> I really hate to add connection semantics via IOCTLs. I would rather
> we
> add them as first class semantics to the char device layer. This
> would
> allow us to use char devices for VNC, for instance.
>
Ok, that's actually what I wanted to do at first, how about:
diff --git a/qemu-char.h b/qemu-char.h
index 1df53ae..22973cd 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -16,6 +16,8 @@
#define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
#define CHR_EVENT_CLOSED 5 /* connection closed */
+#define CHR_DEVICE_EVENT_OPENED 0
+#define CHR_DEVICE_EVENT_CLOSED 1
#define CHR_IOCTL_SERIAL_SET_PARAMS 1
typedef struct {
@@ -59,6 +61,7 @@ struct CharDriverState {
int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
void (*chr_update_read_handler)(struct CharDriverState *s);
int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
+int (*chr_device_event)(struct CharDriverState *s, int cmd, void *arg);
int (*get_msgfd)(struct CharDriverState *s);
IOEventHandler *chr_event;
IOCanReadHandler *chr_can_read;
@@ -83,6 +86,7 @@ void qemu_chr_close(CharDriverState *chr);
void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
void qemu_chr_send_event(CharDriverState *s, int event);
+void qemu_chr_send_device_event(CharDriverState *s, int event, void *arg);
void qemu_chr_add_handlers(CharDriverState *s,
IOCanReadHandler *fd_can_read,
IOReadHandler *fd_read,
> Regards,
>
> Anthony Liguori
>
> > Alon
> >
> > -- commit message
> > From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:00
> 2001
> > From: Alon Levy
> > Date: Mon, 2 Aug 2010 11:22:58 +0300
> > Subject: [PATCH] virtio-console: add IOCTL's for
> guest_{ready,open,close}
> >
> > Add three IOCTL corresponding to the three control events of:
> > guest_ready -> CHR_IOCTL_VIRT_SERIAL_READY
> > guest_open -> CHR_IOCTL_VIRT_SERIAL_OPEN
> > guest_close -> CHR_IOCTL_VIRT_SERIAL_CLOSE
> >
> > Can be used by a matching backend.
> > ---
> > hw/virtio-console.c | 33 +
> > qemu-char.h |4
> > 2 files changed, 37 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> > index caea11f..4c3686d 100644
> > --- a/hw/virtio-console.c
> > +++ b/hw/virtio-console.c
> > @@ -58,6 +58,33 @@ static void chr_event(void *opaque, int event)
> > }
> > }
> >
> > +static void virtconsole_guest_open(VirtIOSerialPort *port)
> > +{
> > +VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> > +
> > +if (vcon->chr) {
> > +qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_OPEN,
> NULL);
> > +}
> > +}
> > +
> > +static void virtconsole_guest_close(VirtIOSerialPort *port)
> > +{
> > +VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> > +
> > +if (vcon->chr) {
> > +qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_CLOSE,
> NULL);
> > +}
> > +}
> > +
> > +static void virtconsole_guest_ready(VirtIOSerialPort *port)
> > +{
> > +VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> > +
> > +if (vcon->chr) {
> > +qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_READY,
> NULL);
> > +}
> > +}
> > +
> > /* Virtio Console Ports */
> > static int virtconsole_initfn(VirtIOSerialDevice *dev)
> > {
> > @@ -94,6 +121,9 @@ static VirtIOSerialPortInfo virtconsole_info = {
> > .qdev.size = sizeof(VirtConsole),
> > .init = virtconsole_initfn,
> > .exit = virtconsole_exitfn,
> > +.guest_open= virtconsole_guest_open,
> > +.guest_close = virtconsole_guest_close,
> > +.guest_ready = virtconsole_guest_ready,
> > .qdev.props = (Property[]) {
> > DEFINE_PROP_UINT8("is_console", VirtConsole,
> port.is_console, 1),
> > DEFINE_PROP_UINT32("nr", VirtConsole, port.id,
> VIRTIO_CONSOLE_BAD_ID),
> > @@ -130,6 +160,9 @@ static VirtIOSerialPortInfo virtserialport_info
> = {
> > .qdev.size = sizeo
[Qemu-devel] [Bug 586175] Re: Windows XP/2003 doesn't boot
I had the same problem. I.ve tried with VirtualBox and KVM: Win Xp SP3 hang on the same point (mup.sys when "safe mode")... Both has the same problem I believe the libvirt maybe the cause. So I use "Raw Access" with VirtualBox that solved my problem 00:00:01.385 [/Devices/piix3ide/0/LUN#0/AttachedDriver/Config/] (level 6) 00:00:01.385 Format = "VMDK" (cb=5) 00:00:01.385 Path = "/home/jtloni/.VirtualBox/HardDisks/xp3.vmdk" (cb=45) hope will help.. -- Windows XP/2003 doesn't boot https://bugs.launchpad.net/bugs/586175 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Status in “qemu-kvm” package in Ubuntu: New Status in Debian GNU/Linux: New Status in Fedora: Unknown Bug description: Hello everyone, my qemu doesn't boot any Windows XP/2003 installations if I try to boot the image. If I boot the install cd first, it's boot manager counts down and triggers the boot on it's own. That's kinda stupid. I'm using libvirt, but even by a simple > qemu-kvm -drive file=image.img,media=disk,if=ide,boot=on it won't boot. Qemu hangs at the message "Booting from Hard Disk..." I'm using qemu-kvm-0.12.4 with SeaBIOS 0.5.1 on Gentoo (No-Multilib and AMD64). It's a server, that means I'm using VNC as the primary graphic output but i don't think it should be an issue.
[Qemu-devel] [PATCH] Resend-3: target-arm: Handle 'smc' as an undefined instruction
Handle smc as an undefined instruction instead of having it wrongly interpreted as some other one. Signed-off-by: Adam Lackorzynski --- target-arm/translate.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 6fcdd7e..9b5d650 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -6346,7 +6346,11 @@ static void disas_arm_insn(CPUState * env, DisasContext *s) dead_tmp(tmp2); store_reg(s, rd, tmp); break; -case 7: /* bkpt */ +case 7: +/* SMC? */ +if ((insn & 0xfff0) == 0xe1600070) + goto illegal_op; +/* bkpt */ gen_set_condexec(s); gen_set_pc_im(s->pc - 4); gen_exception(EXCP_BKPT); -- 1.7.1
Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present
On 08/02/2010 11:47 AM, Kevin O'Connor wrote: On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote: On 08/02/2010 11:11 AM, Kevin O'Connor wrote: Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms directory. Sorry, I should have provided this in the first response. Does the bootsplash cause a delay in startup time? If so, we'll want to be able to disable this at boot time via the -boot option. If it doesn't, this patch is probably fine as is. That's okay - the patch is intended to start discussion anyway. There is no added delay for the bootsplash. However, it does take time to decompress it. On my machine it can take between 200-300ms for the jpeg code. It may make more sense for SeaBIOS to only bother with the bootsplash if the boot menu is enabled (or some new option) - as otherwise the picture is basically cleared immiediately after it's shown. Yeah, if there's no boot menu and no boot menu delay, I imagine the boot splash ends up just being a blip on the screen, right? We definitely want a "boot as fast as you possibly can" mode for the BIOS although for non-developer environments, I think a boot splash is a nice touch. BTW, we need to document somewhere any assumptions SeaBIOS has about the JPEG. I see that it expects a 1024x768 image. Any additional restrictions on the jpeg image? Regards, Anthony Liguori -Kevin
Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present
On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote: > On 08/02/2010 11:11 AM, Kevin O'Connor wrote: > >Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms > >directory. > > Sorry, I should have provided this in the first response. > > Does the bootsplash cause a delay in startup time? If so, we'll > want to be able to disable this at boot time via the -boot option. > If it doesn't, this patch is probably fine as is. That's okay - the patch is intended to start discussion anyway. There is no added delay for the bootsplash. However, it does take time to decompress it. On my machine it can take between 200-300ms for the jpeg code. It may make more sense for SeaBIOS to only bother with the bootsplash if the boot menu is enabled (or some new option) - as otherwise the picture is basically cleared immiediately after it's shown. -Kevin
Re: [Qemu-devel] Netboot happens twice (first fail) when using etherboot ROMs
02.08.2010 20:23, Gianni Tedesco wrote: > On Sun, 2010-08-01 at 21:27 +0100, Stefan Hajnoczi wrote: >> On Sun, Aug 1, 2010 at 10:44 AM, Michael Tokarev wrote: >>> I wonder why with etherboot ROMs, the network boot >>> happens two times (0.12.x), like this: >>> >>> - >>> Starting SeaBIOS (version 0.5.1-20100801_125707-gandalf) >>> >>> Booting from virtio-net.zrom 5.4.4 (GPL) ether... >>> ROM segment 0xc900 length 0x8000 reloc 0x >>> Etherboot 5.4.4 (GPL) http://etherboot.org >>> Drivers: VIRTIO-NET Images: NBI ELF PXE Exports: PXE >>> Protocols: DHCP TFTP >>> Relocating _text from: [00087780,0009f310) to [07ee8470,07f0) >>> Boot from (N)etwork or (Q)uit? >>> >>> Probing pci nic... >>> Probing isa nic... >>> >>> Boot from (N)etwork or (Q)uit? >>> >>> Probing pci nic... >>> [virtio-net]I/O address 0xc020, IRQ #11 >>> MAC address 52:54:00:12:34:56 >>> Searching for server (DHCP) >>> - >>> >>> The boot ROMs supplied with the source works as expected. >>> >>> What's the difference? Is gpxe behaves differently, or >>> is that a difference in build options? >> >> Etherboot and gPXE are different codebases. gPXE was forked from >> Etherboot around 2005 and is now very different. The Etherboot and >> gPXE behavior cannot always be compared, they are not meant to work >> the same. I never supposed it should work the same. I wondered why etherboot shows somewhat weird behavour. > The BIOS boot protocol allows to continue from next device if an earlier > boot method doesn't work. In the real hardware the ROM typically only > supports one type of card. Therefore a ROM ought to be loaded and tried > from each NIC until one of the attempts successfully boots or there are > no more to try. That's quite understandable. But I _guess_ it's a bit more than that, like, just from the wild, different paths or busses to the _same_ device (say, PCI and ISA). > Anyway, you will need to post more info on your setup: what and how many > NIC's, where the ROM binaries came from... There's no info to post. Standard qemu-kvm (or qemu) 0.12.[45], and standard etherboot boot rom (from rom-o-matic will do too), without any extra options. Only one NIC in a virtual machine, like this: kvm -net nic,model=rtl8139 -net tap -boot n (no other options; model does not matter as long as the right pxe ROM can be found). The boot ROMs are searched in, depending on the qemu/kvm configuration, /usr/share/kvm/. If I put gpxe ones in there, I see only one, successful, attempt to boot. If I put etherboot rom, there will be two attempts, first unsuccessful. Not that it is a big issue, just... weird, and annoying -- in Debian for example we (re)build boot ROMs during package build instead of using the ones supplied in the source tarball, and currently gpxe isn't packages in debian, but etherboot behaves somewhat erratically (but works in the end), so I wondered what the issue is. Thanks! /mjt
Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present
On 08/02/2010 11:11 AM, Kevin O'Connor wrote:
Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
directory.
Sorry, I should have provided this in the first response.
Does the bootsplash cause a delay in startup time? If so, we'll want to
be able to disable this at boot time via the -boot option. If it
doesn't, this patch is probably fine as is.
Regards,
Anthony Liguori
Signed-off-by: Kevin O'Connor
---
Changes v2->v3:
Fix coding style (missing braces).
Changes v1->v2:
Add signed-off-by line.
---
hw/fw_cfg.c |9 +++--
hw/pc.c |8
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 72866ae..5a2c0aa 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -304,8 +304,13 @@ int fw_cfg_add_file(FWCfgState *s, const char *dir, const
char *filename,
basename = filename;
}
-snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
- "%s/%s", dir, basename);
+if (dir&& dir[0]) {
+snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
+ "%s/%s", dir, basename);
+} else {
+strncpy(s->files->f[index].name, basename,
+sizeof(s->files->f[index].name));
+}
for (i = 0; i< index; i++) {
if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
diff --git a/hw/pc.c b/hw/pc.c
index 58dea57..6893799 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -54,6 +54,7 @@
#endif
#define BIOS_FILENAME "bios.bin"
+#define BOOTSPLASH_FILENAME "bootsplash.jpg"
#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
@@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size,
fw_cfg = bochs_bios_init();
rom_set_fw(fw_cfg);
+/* Optional bootsplash file */
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME);
+if (filename) {
+qemu_free(filename);
+rom_add_file(BOOTSPLASH_FILENAME, "", 0);
+}
+
if (linux_boot) {
load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline,
below_4g_mem_size);
}
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On 08/02/2010 11:29 AM, Blue Swirl wrote: Yeah, I'm neglecting the fact that we're not consistent as maintainers and I'm all for dropping it from CODING_STYLE. I'd rather expand the document. For example, I like the approach libvirt takes: http://libvirt.org/git/?p=libvirt.git;a=blob_plain;f=HACKING;hb=HEAD Not specifically to braces, but they describe types and memory allocation etc. which is FAQ stuff for us too. I agree 100%. There are a lot of idioms in QEMU that would be good to document for people learning the code base. But I think that's orthogonal to the discussion of whether we want to keep the if rule and whether we want to enforce all of coding style at commit time. Regards, Anthony Liguori
Re: [Qemu-devel] Netboot happens twice (first fail) when using etherboot ROMs
On Sun, 2010-08-01 at 21:27 +0100, Stefan Hajnoczi wrote: > On Sun, Aug 1, 2010 at 10:44 AM, Michael Tokarev wrote: > > I wonder why with etherboot ROMs, the network boot > > happens two times (0.12.x), like this: > > > > - > > Starting SeaBIOS (version 0.5.1-20100801_125707-gandalf) > > > > Booting from virtio-net.zrom 5.4.4 (GPL) ether... > > ROM segment 0xc900 length 0x8000 reloc 0x > > Etherboot 5.4.4 (GPL) http://etherboot.org > > Drivers: VIRTIO-NET Images: NBI ELF PXE Exports: PXE > > Protocols: DHCP TFTP > > Relocating _text from: [00087780,0009f310) to [07ee8470,07f0) > > Boot from (N)etwork or (Q)uit? > > > > Probing pci nic... > > Probing isa nic... > > > > Boot from (N)etwork or (Q)uit? > > > > Probing pci nic... > > [virtio-net]I/O address 0xc020, IRQ #11 > > MAC address 52:54:00:12:34:56 > > Searching for server (DHCP) > > - > > > > The boot ROMs supplied with the source works as expected. > > > > What's the difference? Is gpxe behaves differently, or > > is that a difference in build options? > > Etherboot and gPXE are different codebases. gPXE was forked from > Etherboot around 2005 and is now very different. The Etherboot and > gPXE behavior cannot always be compared, they are not meant to work > the same. The BIOS boot protocol allows to continue from next device if an earlier boot method doesn't work. In the real hardware the ROM typically only supports one type of card. Therefore a ROM ought to be loaded and tried from each NIC until one of the attempts successfully boots or there are no more to try. Anyway, you will need to post more info on your setup: what and how many NIC's, where the ROM binaries came from... Gianni Tedesco
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On Mon, Aug 2, 2010 at 4:18 PM, Anthony Liguori wrote: > On 08/02/2010 11:06 AM, malc wrote: >> >> On Mon, 2 Aug 2010, Anthony Liguori wrote: >> >> >>> >>> On 08/02/2010 10:41 AM, Kevin Wolf wrote: >>> > > But something like braces around an if doesn't seem like it creates a > big problem. Most C programmers are used to seeing braces in some > statements and not other. Therefore, it's hard to argue that the code > gets really unreadable if this isn't strictly enforced. > > I won't argue that missing braces impact readability of the code, they probably don't. However, as was pointed out in earlier discussion there still remain two important points: 1. While it doesn't make a difference for the code itself, readability of patches suffers when braces have to be added/removed when a second line is inserted or disappears. >>> >>> I understand the argument but it's not something that I strongly agree >>> with. >>> >>> 2. I've messed up more than once with adding some debug code (even worse when it happens with real code): if (foo) fprintf(stderr, "debug msg\n"); bar(); /* oops, not conditional any more */ >>> >>> This is hard to do with editors that auto indent unless you're copying >>> and >>> pasting debugging. And yeah, I've made that mistake too doing the later >>> :-) >>> >>> This is why I tend to disagree with removing the rule, and suggest to rather implement some automatic checks like Aurelien suggested (if we need to change anything at all). I usually don't ask for a respin just for braces if the patch is good otherwise, but if you think we should just reject such patches without exceptions, I can change that. >>> >>> Yeah, I try to remember to enforce it but often forget or just don't >>> notice. >>> My eyes don't tend to catch missing braces like they would catch bad type >>> naming because the former is really not uncommon. >>> >>> I'm happy with the status quo but I won't object to a git commit hook >>> that >>> enforces style. >>> >> >> Seriously? You are happy with the situation where some people get their >> patches rejected because they disagree/forogot/don't_care about single >> statement braces while the patches of others make it through? >> > > Yeah, I'm neglecting the fact that we're not consistent as maintainers and > I'm all for dropping it from CODING_STYLE. I'd rather expand the document. For example, I like the approach libvirt takes: http://libvirt.org/git/?p=libvirt.git;a=blob_plain;f=HACKING;hb=HEAD Not specifically to braces, but they describe types and memory allocation etc. which is FAQ stuff for us too.
[Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present
Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
directory.
Signed-off-by: Kevin O'Connor
---
Changes v2->v3:
Fix coding style (missing braces).
Changes v1->v2:
Add signed-off-by line.
---
hw/fw_cfg.c |9 +++--
hw/pc.c |8
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 72866ae..5a2c0aa 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -304,8 +304,13 @@ int fw_cfg_add_file(FWCfgState *s, const char *dir, const
char *filename,
basename = filename;
}
-snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
- "%s/%s", dir, basename);
+if (dir && dir[0]) {
+snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
+ "%s/%s", dir, basename);
+} else {
+strncpy(s->files->f[index].name, basename,
+sizeof(s->files->f[index].name));
+}
for (i = 0; i < index; i++) {
if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
diff --git a/hw/pc.c b/hw/pc.c
index 58dea57..6893799 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -54,6 +54,7 @@
#endif
#define BIOS_FILENAME "bios.bin"
+#define BOOTSPLASH_FILENAME "bootsplash.jpg"
#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
@@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size,
fw_cfg = bochs_bios_init();
rom_set_fw(fw_cfg);
+/* Optional bootsplash file */
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME);
+if (filename) {
+qemu_free(filename);
+rom_add_file(BOOTSPLASH_FILENAME, "", 0);
+}
+
if (linux_boot) {
load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline,
below_4g_mem_size);
}
--
1.7.2
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On 08/02/2010 11:06 AM, malc wrote: On Mon, 2 Aug 2010, Anthony Liguori wrote: On 08/02/2010 10:41 AM, Kevin Wolf wrote: But something like braces around an if doesn't seem like it creates a big problem. Most C programmers are used to seeing braces in some statements and not other. Therefore, it's hard to argue that the code gets really unreadable if this isn't strictly enforced. I won't argue that missing braces impact readability of the code, they probably don't. However, as was pointed out in earlier discussion there still remain two important points: 1. While it doesn't make a difference for the code itself, readability of patches suffers when braces have to be added/removed when a second line is inserted or disappears. I understand the argument but it's not something that I strongly agree with. 2. I've messed up more than once with adding some debug code (even worse when it happens with real code): if (foo) fprintf(stderr, "debug msg\n"); bar(); /* oops, not conditional any more */ This is hard to do with editors that auto indent unless you're copying and pasting debugging. And yeah, I've made that mistake too doing the later :-) This is why I tend to disagree with removing the rule, and suggest to rather implement some automatic checks like Aurelien suggested (if we need to change anything at all). I usually don't ask for a respin just for braces if the patch is good otherwise, but if you think we should just reject such patches without exceptions, I can change that. Yeah, I try to remember to enforce it but often forget or just don't notice. My eyes don't tend to catch missing braces like they would catch bad type naming because the former is really not uncommon. I'm happy with the status quo but I won't object to a git commit hook that enforces style. Seriously? You are happy with the situation where some people get their patches rejected because they disagree/forogot/don't_care about single statement braces while the patches of others make it through? Yeah, I'm neglecting the fact that we're not consistent as maintainers and I'm all for dropping it from CODING_STYLE. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On Mon, Aug 2, 2010 at 3:20 PM, Anthony Liguori wrote: > On 07/31/2010 06:49 PM, Aurelien Jarno wrote: >> >> On Sat, Jul 31, 2010 at 08:23:55PM +, Blue Swirl wrote: >> >>> >>> On Sat, Jul 31, 2010 at 4:23 PM, malc wrote: >>> History has shown that this particular rule is unenforcable. Signed-off-by: malc --- CODING_STYLE | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) >>> >>> Not again: >>> http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html >>> >>> There are plenty of ways to make the rule enforceable, for example we >>> could agree to start to revert commits which introduce new >>> CODING_STYLE violations. >>> >>> >> >> It seems to be possible to add a pre-applypatch script to the git hook >> directory, that will verify the commit and reject it if it doesn't >> comply with the coding rules. Of course it's possible to commit a patch >> anyway by using --no-verify. >> > > There are certain aspects of CODING_STYLE that I think are pretty important. > For instance, space vs. tabs can really screw things up for people that > have non-standard tabs. This is something that enforcing at patch > submission time seems to be really important. > > Type naming seems important too because it's often not isolated. IOW, a > poor choice in one file can end up polluting other files quickly that > require interacting. The result is a mess of naming styles. > > But something like braces around an if doesn't seem like it creates a big > problem. Most C programmers are used to seeing braces in some statements > and not other. Therefore, it's hard to argue that the code gets really > unreadable if this isn't strictly enforced. > > So really, I think the problem is that we're enforcing the words of > CODING_STYLE instead of the intent. The intent of CODING_STYLE is to > improve the readability of the code. I think it requires a certain amount > of taste to be applied. The problem with that approach is that CODING_STYLE is not written like that at all. All four requirements equally state that they describe QEMU coding style. There is also no room left for interpretation: "Every indented statement is braced; even if the block contains just one statement."
Re: [Qemu-devel] [PATCH 00/20] MIPS Magnum conversion to qdev
On Sun, Aug 1, 2010 at 1:42 PM, Hervé Poussineau wrote: > This series converts devices used by MIPS Magnum emulation to qdev devices. > Once applied, Magnum emulation will be fully creatable by a configuration > file (see attached file) > > usage: > qemu-system-mips64el -M empty -nodefaults -readconfig magnum > -netdev id=nic,type=user > -drive id=disk,format=qcow2,if=none,file=1G.qcow2 > -drive id=cdrom,media=cdrom,format=raw,if=none,file=arccd.iso > -chardev id=serial0,backend=vc > -chardev id=serial1,backend=vc > -chardev id=parallel0,backend=vc > > All feedback is very appreciated. I appreciate very much your goal to make the devices qdevified. However, the approach is not correct. Instead of adding 'iobase' parameters and using cpu_register_physical_memory() in the device code, you should just use sysbus_mmio_map() at board level. I also don't see much need for a dedicated rc4030 bus. See for example syborg.c for a mostly qdevified board.
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On Mon, 2 Aug 2010, Anthony Liguori wrote: > On 08/02/2010 10:41 AM, Kevin Wolf wrote: > > > > > But something like braces around an if doesn't seem like it creates a > > > big problem. Most C programmers are used to seeing braces in some > > > statements and not other. Therefore, it's hard to argue that the code > > > gets really unreadable if this isn't strictly enforced. > > > > > I won't argue that missing braces impact readability of the code, they > > probably don't. However, as was pointed out in earlier discussion there > > still remain two important points: > > > > 1. While it doesn't make a difference for the code itself, readability > > of patches suffers when braces have to be added/removed when a second > > line is inserted or disappears. > > > > I understand the argument but it's not something that I strongly agree with. > > > 2. I've messed up more than once with adding some debug code (even worse > > when it happens with real code): > > > > if (foo) > > fprintf(stderr, "debug msg\n"); > > bar(); /* oops, not conditional any more */ > > > > This is hard to do with editors that auto indent unless you're copying and > pasting debugging. And yeah, I've made that mistake too doing the later :-) > > > This is why I tend to disagree with removing the rule, and suggest to > > rather implement some automatic checks like Aurelien suggested (if we > > need to change anything at all). I usually don't ask for a respin just > > for braces if the patch is good otherwise, but if you think we should > > just reject such patches without exceptions, I can change that. > > > > Yeah, I try to remember to enforce it but often forget or just don't notice. > My eyes don't tend to catch missing braces like they would catch bad type > naming because the former is really not uncommon. > > I'm happy with the status quo but I won't object to a git commit hook that > enforces style. Seriously? You are happy with the situation where some people get their patches rejected because they disagree/forogot/don't_care about single statement braces while the patches of others make it through? -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCHv2] Load "bootsplash.jpg" if present
On 08/02/2010 10:32 AM, Kevin O'Connor wrote:
Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
directory.
Signed-off-by: Kevin O'Connor
---
Changes v1->v2:
Add signed-off-by line.
---
hw/fw_cfg.c |8 ++--
hw/pc.c |8
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 72866ae..c4319d7 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -304,8 +304,12 @@ int fw_cfg_add_file(FWCfgState *s, const char *dir, const
char *filename,
basename = filename;
}
-snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
- "%s/%s", dir, basename);
+if (dir&& dir[0])
+snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
+ "%s/%s", dir, basename);
+else
+strncpy(s->files->f[index].name, basename,
+sizeof(s->files->f[index].name));
Missing {}s around the ifs :-)
Regards,
Anthony Liguori
for (i = 0; i< index; i++) {
if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
diff --git a/hw/pc.c b/hw/pc.c
index 58dea57..6893799 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -54,6 +54,7 @@
#endif
#define BIOS_FILENAME "bios.bin"
+#define BOOTSPLASH_FILENAME "bootsplash.jpg"
#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
@@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size,
fw_cfg = bochs_bios_init();
rom_set_fw(fw_cfg);
+/* Optional bootsplash file */
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME);
+if (filename) {
+qemu_free(filename);
+rom_add_file(BOOTSPLASH_FILENAME, "", 0);
+}
+
if (linux_boot) {
load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline,
below_4g_mem_size);
}
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On 08/02/2010 10:55 AM, malc wrote: Rejecting a big patch because braces aren't used in single line if statements seems to be an unnecessary barrier to me. Taking such patches anyway is basically what we're doing today, right? And what malc is complaining about. Malc is mainly complaining about "Quod licet Jovi non licet bovi" situation. Very fair point. Regards, Anthony Liguori
[Qemu-devel] Re: [TUHS & QEMU] Making progress with old DG/UX virtualization. Need advice.
Hi, El 02/08/2010, a las 08:48, DG UX escribió: > Thanks Natalia, > > I'll start by answering the insultive part of your answer, as my ego > will not let me go on if I don't: > > I am not "begging on all the internet", I am simply seeking solutions, > help and advice, and making sure to update whoever is interested in > the progress I am doing. > Also, I wish to thank you for your insight and well detailed answer. > Finally I got an explanation as to _why_ solution A will not be as > good as solution B. That is what I call a winning argument, and I > thank you for that. That's why I sent you the message not because egos > I already have people searching for Adaptec docs and programmers for > the creation of the driver, err, emulated device. Great, I wish you my best and offer my repository of operating systems to test the emulated device on as much systems as possible when it is mature enough. > Take care, > Adam Natalia Portillo Claunia.com > On Mon, Aug 2, 2010 at 9:11 AM, Natalia Portillo wrote: >> Hi, >> >> I've read all your posts in the QEMU mailing list and the TUHS one and I'm >> answering to both lists in a hope my mail enlights you and any other curious. >> >> First of all, old UNIX systems (and I put my hand on the fire for DG/UX >> also), use a monolithic linked at setup/later time kernel. >> That is, even if you get a driver (IDE, virtio, whatsoever), the >> configuration files, the kernel, the ramdisk, everything that lets your >> system boot, MUST HAVE BEEN BOOT from the AIC controller, the driver is >> hardcoded, no way to change it. >> >> If you have extensive knowledge of what files a driver setup modifies on >> DG/UX specifically (knowledge from other UNIX, forget it, they are as >> different as Porsche and Ferrari motors), you can always get a new kernel >> with the drivers you need to make it boot and manually put them in your >> image. >> >> In the case, you meet this requirements, and, you do it, you can then >> achieve to other problems. The DG/UX workstations are x86 machines, but >> nothing swears they are PC compatible machines, and they can have a >> different memory map for some critical device, or include critical devices >> never found in a PC (like an Intel Macintosh does for example). Just booting >> from a BIOS doesn't make the machines be the same (PowerPC Macintosh, IBM >> POWER workstations, Genesi Pegasos, are machines that boot OpenFirmware with >> heavily different configurations, devices and memory maps). >> >> Also, you are assuming IDE is available in DG/UX just because the controller >> is present in the hardware. That hardware was also used for Windows NT. IDE >> support can be JUST FOR Windows, and the DG/UX manufacturer just decided to >> not include an IDE driver in the kernel (happened in AIX for PCs until last >> version of all, only SCSI was supported, being a hugely strange controller >> in PC worlds). >> >> In the case you opt for making a driver (adding IDE, virtio, or other SCSI >> support) for the DG/UX need to say you need, low level knowledge of the >> hardware, low level knowledge of the operating system, a working machine >> (for sure, with the hardware available), a debug machine (almost sure also), >> C and maybe assembler knowledge. In a scale of 10, this puts the difficulty >> in 8 for most of programmers, and surely if you were one you stacked with >> the first option everyone gave you (see next sentence). >> >> The easiest way, and the one that people answered you already in QEMU's >> mailing list (in a scale of 10 the difficulty is 6 or even 5), is creating >> an emulated device (that's the correct term, not "driver") for an emulator, >> like QEMU, Bochs, VirtualBox (forget this option for VMWare, VirtualPC or >> Parallels) that adds the AIC SCSI controller you exactly need. >> >> Why is this easiest? You don't need any DG/UX working system, you don't need >> to know how DG/UX works, you don't need to compile a kernel, copy it to your >> image. >> >> You just take the Adaptec's documentation, and start coding, making a SCSI >> emulated controller, and testing it with systems you can always reinstall, >> debug, and check, until they fully work (Windows, Linux, BSD, take your >> choice). >> >> And then, you just polish it until your DG/UX boots, or finds the memory map >> as a mess it doesnt like. >> >> Finally, please stop begging on all the internet, spend that time coding the >> driver or getting the money to pay a programmer that will do. >> >> Sincerely yours, >> Natalia Portillo >> Claunia.com CEO >> QEMU's Official OS Support List maintainer
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On Mon, 2 Aug 2010, Kevin Wolf wrote: > Am 02.08.2010 17:20, schrieb Anthony Liguori: > > On 07/31/2010 06:49 PM, Aurelien Jarno wrote: > >> On Sat, Jul 31, 2010 at 08:23:55PM +, Blue Swirl wrote: [..snip..] > > > So really, I think the problem is that we're enforcing the words of > > CODING_STYLE instead of the intent. The intent of CODING_STYLE is to > > improve the readability of the code. I think it requires a certain > > amount of taste to be applied. > > > Rejecting a big patch because braces aren't used in single line if > > statements seems to be an unnecessary barrier to me. > > Taking such patches anyway is basically what we're doing today, right? > And what malc is complaining about. Malc is mainly complaining about "Quod licet Jovi non licet bovi" situation. -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On 08/02/2010 10:41 AM, Kevin Wolf wrote: But something like braces around an if doesn't seem like it creates a big problem. Most C programmers are used to seeing braces in some statements and not other. Therefore, it's hard to argue that the code gets really unreadable if this isn't strictly enforced. I won't argue that missing braces impact readability of the code, they probably don't. However, as was pointed out in earlier discussion there still remain two important points: 1. While it doesn't make a difference for the code itself, readability of patches suffers when braces have to be added/removed when a second line is inserted or disappears. I understand the argument but it's not something that I strongly agree with. 2. I've messed up more than once with adding some debug code (even worse when it happens with real code): if (foo) fprintf(stderr, "debug msg\n"); bar(); /* oops, not conditional any more */ This is hard to do with editors that auto indent unless you're copying and pasting debugging. And yeah, I've made that mistake too doing the later :-) This is why I tend to disagree with removing the rule, and suggest to rather implement some automatic checks like Aurelien suggested (if we need to change anything at all). I usually don't ask for a respin just for braces if the patch is good otherwise, but if you think we should just reject such patches without exceptions, I can change that. Yeah, I try to remember to enforce it but often forget or just don't notice. My eyes don't tend to catch missing braces like they would catch bad type naming because the former is really not uncommon. I'm happy with the status quo but I won't object to a git commit hook that enforces style. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
Am 02.08.2010 17:20, schrieb Anthony Liguori: > On 07/31/2010 06:49 PM, Aurelien Jarno wrote: >> On Sat, Jul 31, 2010 at 08:23:55PM +, Blue Swirl wrote: >> >>> On Sat, Jul 31, 2010 at 4:23 PM, malc wrote: >>> History has shown that this particular rule is unenforcable. Signed-off-by: malc --- CODING_STYLE | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) >>> Not again: >>> http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html >>> >>> There are plenty of ways to make the rule enforceable, for example we >>> could agree to start to revert commits which introduce new >>> CODING_STYLE violations. >>> >>> >> It seems to be possible to add a pre-applypatch script to the git hook >> directory, that will verify the commit and reject it if it doesn't >> comply with the coding rules. Of course it's possible to commit a patch >> anyway by using --no-verify. >> > > There are certain aspects of CODING_STYLE that I think are pretty > important. For instance, space vs. tabs can really screw things up for > people that have non-standard tabs. This is something that enforcing at > patch submission time seems to be really important. > > Type naming seems important too because it's often not isolated. IOW, a > poor choice in one file can end up polluting other files quickly that > require interacting. The result is a mess of naming styles. > > But something like braces around an if doesn't seem like it creates a > big problem. Most C programmers are used to seeing braces in some > statements and not other. Therefore, it's hard to argue that the code > gets really unreadable if this isn't strictly enforced. I won't argue that missing braces impact readability of the code, they probably don't. However, as was pointed out in earlier discussion there still remain two important points: 1. While it doesn't make a difference for the code itself, readability of patches suffers when braces have to be added/removed when a second line is inserted or disappears. 2. I've messed up more than once with adding some debug code (even worse when it happens with real code): if (foo) fprintf(stderr, "debug msg\n"); bar(); /* oops, not conditional any more */ This is why I tend to disagree with removing the rule, and suggest to rather implement some automatic checks like Aurelien suggested (if we need to change anything at all). I usually don't ask for a respin just for braces if the patch is good otherwise, but if you think we should just reject such patches without exceptions, I can change that. > So really, I think the problem is that we're enforcing the words of > CODING_STYLE instead of the intent. The intent of CODING_STYLE is to > improve the readability of the code. I think it requires a certain > amount of taste to be applied. > > Rejecting a big patch because braces aren't used in single line if > statements seems to be an unnecessary barrier to me. Taking such patches anyway is basically what we're doing today, right? And what malc is complaining about. Kevin
[Qemu-devel] [PATCHv2] Load "bootsplash.jpg" if present
Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
directory.
Signed-off-by: Kevin O'Connor
---
Changes v1->v2:
Add signed-off-by line.
---
hw/fw_cfg.c |8 ++--
hw/pc.c |8
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 72866ae..c4319d7 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -304,8 +304,12 @@ int fw_cfg_add_file(FWCfgState *s, const char *dir, const
char *filename,
basename = filename;
}
-snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
- "%s/%s", dir, basename);
+if (dir && dir[0])
+snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
+ "%s/%s", dir, basename);
+else
+strncpy(s->files->f[index].name, basename,
+sizeof(s->files->f[index].name));
for (i = 0; i < index; i++) {
if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
diff --git a/hw/pc.c b/hw/pc.c
index 58dea57..6893799 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -54,6 +54,7 @@
#endif
#define BIOS_FILENAME "bios.bin"
+#define BOOTSPLASH_FILENAME "bootsplash.jpg"
#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
@@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size,
fw_cfg = bochs_bios_init();
rom_set_fw(fw_cfg);
+/* Optional bootsplash file */
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME);
+if (filename) {
+qemu_free(filename);
+rom_add_file(BOOTSPLASH_FILENAME, "", 0);
+}
+
if (linux_boot) {
load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline,
below_4g_mem_size);
}
--
1.7.2
[Qemu-devel] [PATCH 2/4] scsi-disk: fix the mode data header returned by the MODE SENSE(10) command
The header for the MODE SENSE(10) command is 8 bytes long.
Signed-off-by: Bernhard Kohl
---
hw/scsi-disk.c | 35 ---
1 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 57439f4..927df54 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -606,6 +606,7 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req,
uint8_t *outbuf)
uint64_t nb_sectors;
int page, dbd, buflen;
uint8_t *p;
+uint8_t dev_specific_param;
dbd = req->cmd.buf[1] & 0x8;
page = req->cmd.buf[2] & 0x3f;
@@ -613,16 +614,31 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req,
uint8_t *outbuf)
memset(outbuf, 0, req->cmd.xfer);
p = outbuf;
-p[1] = 0; /* Default media type. */
-p[3] = 0; /* Block descriptor length. */
-if (bdrv_is_read_only(s->bs)) {
-p[2] = 0x80; /* Readonly. */
+if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM ||
+bdrv_is_read_only(s->bs)) {
+dev_specific_param = 0x80; /* Readonly. */
+} else {
+dev_specific_param = 0x00;
+}
+
+if (req->cmd.buf[0] == MODE_SENSE) {
+p[1] = 0; /* Default media type. */
+p[2] = dev_specific_param;
+p[3] = 0; /* Block descriptor length. */
+p += 4;
+} else { /* MODE_SENSE_10 */
+p[2] = 0; /* Default media type. */
+p[3] = dev_specific_param;
+p[6] = p[7] = 0; /* Block descriptor length. */
+p += 8;
}
-p += 4;
bdrv_get_geometry(s->bs, &nb_sectors);
if ((~dbd) & nb_sectors) {
-outbuf[3] = 8; /* Block descriptor length */
+if (req->cmd.buf[0] == MODE_SENSE)
+outbuf[3] = 8; /* Block descriptor length */
+else /* MODE_SENSE_10 */
+outbuf[7] = 8; /* Block descriptor length */
nb_sectors /= s->cluster_size;
nb_sectors--;
if (nb_sectors > 0xff)
@@ -652,7 +668,12 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req,
uint8_t *outbuf)
}
buflen = p - outbuf;
-outbuf[0] = buflen - 1;
+if (req->cmd.buf[0] == MODE_SENSE) {
+outbuf[0] = buflen - 1;
+} else { /* MODE_SENSE_10 */
+outbuf[0] = ((buflen - 1) >> 8) & 0xff;
+outbuf[1] = (buflen - 1) & 0xff;
+}
if (buflen > req->cmd.xfer)
buflen = req->cmd.xfer;
return buflen;
--
1.7.2
[Qemu-devel] [PATCH 4/4] scsi-disk: fix the block descriptor returned by the MODE SENSE command
The block descriptor contains the number of blocks, not the highest LBA. Real hard disks return 0 if the number of blocks exceed the maximum 0xFF. SCSI-Spec: http://ldkelley.com/SCSI2/SCSI2/SCSI2-08.html#8.3.3 The number of blocks field specifies the number of logical blocks on the medium to which the density code and block length fields apply. A value of zero indicates that all of the remaining logical blocks of the logical unit shall have the medium characteristics specified. Signed-off-by: Bernhard Kohl --- hw/scsi-disk.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 26f7345..519513c 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -642,9 +642,8 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) else /* MODE_SENSE_10 */ outbuf[7] = 8; /* Block descriptor length */ nb_sectors /= s->cluster_size; -nb_sectors--; if (nb_sectors > 0xff) -nb_sectors = 0xff; +nb_sectors = 0; p[0] = 0; /* media density code */ p[1] = (nb_sectors >> 16) & 0xff; p[2] = (nb_sectors >> 8) & 0xff; -- 1.7.2
[Qemu-devel] [PATCH 1/4] scsi-disk: fix the mode data length field returned by the MODE SENSE command
The MODE DATA LENGTH field indicates the length in bytes of the following data that is available to be transferred. The mode data length does not include the number of bytes in the MODE DATA LENGTH field. Signed-off-by: Bernhard Kohl --- hw/scsi-disk.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index f43f2d0..57439f4 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -652,7 +652,7 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf) } buflen = p - outbuf; -outbuf[0] = buflen - 4; +outbuf[0] = buflen - 1; if (buflen > req->cmd.xfer) buflen = req->cmd.xfer; return buflen; -- 1.7.2
[Qemu-devel] [PATCH 3/4] scsi-disk: fix changeable values returned by the MODE SENSE command
If the page control (PC) field in the MODE SENSE command defines Changeable
Values to be returned in the mode pages, don't return any mode page as there
is no support to change any values.
Signed-off-by: Bernhard Kohl
---
hw/scsi-disk.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 927df54..26f7345 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -604,13 +604,15 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req,
uint8_t *outbuf)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
uint64_t nb_sectors;
-int page, dbd, buflen;
+int page, dbd, buflen, page_control;
uint8_t *p;
uint8_t dev_specific_param;
dbd = req->cmd.buf[1] & 0x8;
page = req->cmd.buf[2] & 0x3f;
-DPRINTF("Mode Sense (page %d, len %zd)\n", page, req->cmd.xfer);
+page_control = (req->cmd.buf[2] & 0xc0) >> 6;
+DPRINTF("Mode Sense(%d) (page %d, len %d, page_control %d)\n",
+(req->cmd.buf[0] == MODE_SENSE) ? 6 : 10, page, len, page_control);
memset(outbuf, 0, req->cmd.xfer);
p = outbuf;
@@ -654,7 +656,8 @@ static int scsi_disk_emulate_mode_sense(SCSIRequest *req,
uint8_t *outbuf)
p += 8;
}
-switch (page) {
+/* Don't return pages if Changeable Values (1) are requested. */
+if (page_control != 1) switch (page) {
case 0x04:
case 0x05:
case 0x08:
--
1.7.2
[Qemu-devel] [PATCH 0/4] scsi-disk: improve the emulation of the MODE SENSE command
This series fixes some issues with the MODE SENSE command. I have an OS which fails during this command. It works fine with real SCSI disk hardware. Thanks Bernhard
[Qemu-devel] [PATCHv2] The USB tablet should not claim boot protocol support.
The USB tablet advertises that it supports the "boot" protocol.
However, its reports aren't "boot" protocol compatible. So, it
shouldn't claim that.
Signed-off-by: Kevin O'Connor
---
Changes v1->v2:
Add signed-off-by line.
---
hw/usb-hid.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index d54823d..3af0c5a 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -182,7 +182,7 @@ static const uint8_t qemu_tablet_config_descriptor[] = {
0x00, /* u8 if_bAlternateSetting; */
0x01, /* u8 if_bNumEndpoints; */
0x03, /* u8 if_bInterfaceClass; */
- 0x01, /* u8 if_bInterfaceSubClass; */
+ 0x00, /* u8 if_bInterfaceSubClass; */
0x02, /* u8 if_bInterfaceProtocol; [usb1.1 or single tt] */
0x07, /* u8 if_iInterface; */
--
1.7.2
[Qemu-devel] [PATCHv2] Fix USB mouse Set_Protocol behavior
The QEMU USB mouse claims to support the "boot" protocol (bInterfaceSubClass is 1). However, the mouse rejects the Set_Protocol command. The qemu mouse does support the "boot" protocol specification, so a simple fix is to enable the Set_Protocol request. Signed-off-by: Kevin O'Connor --- Changes v1->v2: Add signed-off-by line. --- hw/usb-hid.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb-hid.c b/hw/usb-hid.c index 882d933..d54823d 100644 --- a/hw/usb-hid.c +++ b/hw/usb-hid.c @@ -791,13 +791,13 @@ static int usb_hid_handle_control(USBDevice *dev, int request, int value, goto fail; break; case GET_PROTOCOL: -if (s->kind != USB_KEYBOARD) +if (s->kind != USB_KEYBOARD && s->kind != USB_MOUSE) goto fail; ret = 1; data[0] = s->protocol; break; case SET_PROTOCOL: -if (s->kind != USB_KEYBOARD) +if (s->kind != USB_KEYBOARD && s->kind != USB_MOUSE) goto fail; ret = 0; s->protocol = value; -- 1.7.2
Re: [Qemu-devel] [PATCH 0/3] Fix broken if statements
> Is there some magic (= tool) which detected these "broken windows" > in hw/loader.c, qemu-io.c and vl.c, or was it just a manual code > review or luck? I used a proprietary static analysis tool called BEAM. http://domino.research.ibm.com/comm/research.nsf/pages/r.da.beam.html It found pages of potential errors, about 80% of which seem valid. Fixing the bugs with obvious fixes seems like a good way for me to learn the qemu code while providing a useful service at the same time. If anybody wants to see the output of the tool (plenty of bugs to go around) please email me off list. Some of the bugs it found, I'm thinking of out of bound array accesses and returning pointers to stack variables, probably have security implications so I'd like to not share those publicly until there are patches to fix them.
Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
On 07/31/2010 06:49 PM, Aurelien Jarno wrote: On Sat, Jul 31, 2010 at 08:23:55PM +, Blue Swirl wrote: On Sat, Jul 31, 2010 at 4:23 PM, malc wrote: History has shown that this particular rule is unenforcable. Signed-off-by: malc --- CODING_STYLE | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) Not again: http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html There are plenty of ways to make the rule enforceable, for example we could agree to start to revert commits which introduce new CODING_STYLE violations. It seems to be possible to add a pre-applypatch script to the git hook directory, that will verify the commit and reject it if it doesn't comply with the coding rules. Of course it's possible to commit a patch anyway by using --no-verify. There are certain aspects of CODING_STYLE that I think are pretty important. For instance, space vs. tabs can really screw things up for people that have non-standard tabs. This is something that enforcing at patch submission time seems to be really important. Type naming seems important too because it's often not isolated. IOW, a poor choice in one file can end up polluting other files quickly that require interacting. The result is a mess of naming styles. But something like braces around an if doesn't seem like it creates a big problem. Most C programmers are used to seeing braces in some statements and not other. Therefore, it's hard to argue that the code gets really unreadable if this isn't strictly enforced. So really, I think the problem is that we're enforcing the words of CODING_STYLE instead of the intent. The intent of CODING_STYLE is to improve the readability of the code. I think it requires a certain amount of taste to be applied. Rejecting a big patch because braces aren't used in single line if statements seems to be an unnecessary barrier to me. Regards, Anthony Liguori The good point of this approach is that the rule is enforced by a script, which is not suppose to make mistakes, and that it can be shared between patch submitters and patch committers: both side can make mistakes and it is always better to know that as early as possible. Of course someone as to translate the coding rules in a set of regular expressions able to catch errors.
[Qemu-devel] [Bug 586175] Re: Windows XP/2003 doesn't boot
Hi Andy When i look at your w7 partition table output, then there seems to be a problem with start/end cylinders. Your first partitions last cylinder is 13, but also the start cylinder of your second partition is 13. two partitions should not share the same cylinder/sector! Something seems to be messed up. I would create a loop device and then use a deep scan with "testdisk" on that loop device. May be it's possible to correct the wrong entrys in the partition table. Cheers Andreas -- Windows XP/2003 doesn't boot https://bugs.launchpad.net/bugs/586175 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Status in “qemu-kvm” package in Ubuntu: New Status in Debian GNU/Linux: New Status in Fedora: Unknown Bug description: Hello everyone, my qemu doesn't boot any Windows XP/2003 installations if I try to boot the image. If I boot the install cd first, it's boot manager counts down and triggers the boot on it's own. That's kinda stupid. I'm using libvirt, but even by a simple > qemu-kvm -drive file=image.img,media=disk,if=ide,boot=on it won't boot. Qemu hangs at the message "Booting from Hard Disk..." I'm using qemu-kvm-0.12.4 with SeaBIOS 0.5.1 on Gentoo (No-Multilib and AMD64). It's a server, that means I'm using VNC as the primary graphic output but i don't think it should be an issue.
Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole
On 08/02/2010 03:33 AM, Alon Levy wrote:
Hi,
This patch adds three CHR_IOCTLs and uses them in virtserial devices, to be
used
by a chardev backend, such as a spice vm channel (spice is a vdi solution).
Basically virtio-serial provides three driver initiated events for guest open
of
a device, guest close, and guest ready (driver port init complete) that before
this
patch are not exposed to the chardev backend.
With the spicevmc backend this is used like this:
qemu -chardev spicevmc,id=vdiport,name=vdiport -device
virtserialport,chardev=vdiport,name=com.redhat.spice.0
I'd appreciate any feedback if this seems the right way to accomplish this,
and
for the numbers I grabbed.
I really hate to add connection semantics via IOCTLs. I would rather we
add them as first class semantics to the char device layer. This would
allow us to use char devices for VNC, for instance.
Regards,
Anthony Liguori
Alon
-- commit message
From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:00 2001
From: Alon Levy
Date: Mon, 2 Aug 2010 11:22:58 +0300
Subject: [PATCH] virtio-console: add IOCTL's for guest_{ready,open,close}
Add three IOCTL corresponding to the three control events of:
guest_ready -> CHR_IOCTL_VIRT_SERIAL_READY
guest_open -> CHR_IOCTL_VIRT_SERIAL_OPEN
guest_close -> CHR_IOCTL_VIRT_SERIAL_CLOSE
Can be used by a matching backend.
---
hw/virtio-console.c | 33 +
qemu-char.h |4
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index caea11f..4c3686d 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -58,6 +58,33 @@ static void chr_event(void *opaque, int event)
}
}
+static void virtconsole_guest_open(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_OPEN, NULL);
+}
+}
+
+static void virtconsole_guest_close(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_CLOSE, NULL);
+}
+}
+
+static void virtconsole_guest_ready(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_READY, NULL);
+}
+}
+
/* Virtio Console Ports */
static int virtconsole_initfn(VirtIOSerialDevice *dev)
{
@@ -94,6 +121,9 @@ static VirtIOSerialPortInfo virtconsole_info = {
.qdev.size = sizeof(VirtConsole),
.init = virtconsole_initfn,
.exit = virtconsole_exitfn,
+.guest_open= virtconsole_guest_open,
+.guest_close = virtconsole_guest_close,
+.guest_ready = virtconsole_guest_ready,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT8("is_console", VirtConsole, port.is_console, 1),
DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
@@ -130,6 +160,9 @@ static VirtIOSerialPortInfo virtserialport_info = {
.qdev.size = sizeof(VirtConsole),
.init = virtserialport_initfn,
.exit = virtconsole_exitfn,
+.guest_open= virtconsole_guest_open,
+.guest_close = virtconsole_guest_close,
+.guest_ready = virtconsole_guest_ready,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR("chardev", VirtConsole, chr),
diff --git a/qemu-char.h b/qemu-char.h
index e3a0783..1df53ae 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -41,6 +41,10 @@ typedef struct {
#define CHR_IOCTL_SERIAL_SET_TIOCM 13
#define CHR_IOCTL_SERIAL_GET_TIOCM 14
+#define CHR_IOCTL_VIRT_SERIAL_OPEN 15
+#define CHR_IOCTL_VIRT_SERIAL_CLOSE 16
+#define CHR_IOCTL_VIRT_SERIAL_READY 17
+
#define CHR_TIOCM_CTS 0x020
#define CHR_TIOCM_CAR 0x040
#define CHR_TIOCM_DSR 0x100
Re: [Qemu-devel] [PATCH 17/20] [MIPS] qdev: convert esp scsi adapter to rc4030 device
2010/8/1 Hervé Poussineau :
> Use it in Jazz emulation
>
> Signed-off-by: Hervé Poussineau
> ---
> hw/esp.c | 83
> +---
> hw/mips_jazz.c | 5 +---
> 2 files changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/hw/esp.c b/hw/esp.c
> index 349052a..d6a9824 100644
> --- a/hw/esp.c
> +++ b/hw/esp.c
> @@ -24,6 +24,8 @@
>
> #include "sysbus.h"
> #include "scsi.h"
> +#include "rc4030.h"
Putting RC4030Device into esp doesn't look clean to me. E.g. Sun
machines don't use RC4030, I'd suggest to keep esp logic machine
independent.
> +#include "qdev-addr.h"
> #include "esp.h"
>
> /* debug ESP card */
> @@ -82,6 +84,15 @@ struct ESPState {
> void *dma_opaque;
> };
>
> +typedef struct RC4030ESPState
> +{
> + RC4030Device dev;
> + target_phys_addr_t iobase;
> + uint32_t irq;
> + uint32_t dma;
> + ESPState state;
> +} RC4030ESPState;
> +
> #define ESP_TCLO 0x0
> #define ESP_TCMID 0x1
> #define ESP_FIFO 0x2
> @@ -356,11 +367,9 @@ static void esp_do_dma(ESPState *s)
> }
> }
>
> -static void esp_command_complete(SCSIBus *bus, int reason, uint32_t tag,
> - uint32_t arg)
> +static void esp_command_complete1(ESPState *s, int reason, uint32_t tag,
> + uint32_t arg)
> {
> - ESPState *s = DO_UPCAST(ESPState, busdev.qdev, bus->qbus.parent);
> -
> if (reason == SCSI_REASON_DONE) {
> DPRINTF("SCSI Command complete\n");
> if (s->ti_size != 0)
> @@ -388,6 +397,20 @@ static void esp_command_complete(SCSIBus *bus, int
> reason, uint32_t tag,
> }
> }
>
> +static void esp_command_complete(SCSIBus *bus, int reason, uint32_t tag,
> + uint32_t arg)
> +{
> + ESPState *s = DO_UPCAST(ESPState, busdev.qdev, bus->qbus.parent);
> + esp_command_complete1(s, reason, tag, arg);
> +}
> +
> +static void esp_rc4030_command_complete(SCSIBus *bus, int reason,
> + uint32_t tag, uint32_t arg)
> +{
> + RC4030ESPState *rc4030 = container_of(bus->qbus.parent, RC4030ESPState,
> dev.qdev);
> + esp_command_complete1(&rc4030->state, reason, tag, arg);
> +}
> +
> static void handle_ti(ESPState *s)
> {
> uint32_t dmalen, minlen;
> @@ -435,6 +458,12 @@ static void esp_hard_reset(DeviceState *d)
> s->rregs[ESP_CFG1] = 7;
> }
>
> +static void esp_rc4030_hard_reset(DeviceState *d)
> +{
> + RC4030ESPState *s = container_of(d, RC4030ESPState, dev.qdev);
> + esp_hard_reset(&s->state.busdev.qdev);
> +}
> +
> static void esp_soft_reset(DeviceState *d)
> {
> ESPState *s = container_of(d, ESPState, busdev.qdev);
> @@ -682,6 +711,27 @@ static int esp_init1(SysBusDevice *dev)
> return scsi_bus_legacy_handle_cmdline(&s->bus);
> }
>
> +static int esp_rc4030_init1(RC4030Device *dev)
> +{
> + RC4030ESPState *rc4030 = container_of(dev, RC4030ESPState, dev);
> + ESPState *s = &rc4030->state;
> + int io;
> +
> + rc4030_init_irq(&rc4030->dev, &s->irq, rc4030->irq);
> +
> + io = cpu_register_io_memory(esp_mem_read, esp_mem_write, s);
> + cpu_register_physical_memory(rc4030->iobase, ESP_REGS << s->it_shift,
> io);
> +
> + qdev_init_gpio_in(&dev->qdev, parent_esp_reset, 1);
> +
> + s->dma_memory_read = rc4030_dma_read;
> + s->dma_memory_write = rc4030_dma_write;
> + s->dma_opaque = rc4030_get_dma(rc4030->dma);
> +
> + scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS,
> esp_rc4030_command_complete);
> + return scsi_bus_legacy_handle_cmdline(&s->bus);
> +}
> +
> static SysBusDeviceInfo esp_info = {
> .init = esp_init1,
> .qdev.name = "esp",
> @@ -693,9 +743,34 @@ static SysBusDeviceInfo esp_info = {
> }
> };
>
> +static const VMStateDescription vmstate_rc4030_esp = {
> + .name = "rc4030-esp",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField []) {
> + VMSTATE_STRUCT(state, RC4030ESPState, 0, vmstate_esp, ESPState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static RC4030DeviceInfo esp_rc4030_info = {
> + .qdev.name = "rc4030-esp",
> + .qdev.size = sizeof(RC4030ESPState),
> + .qdev.vmsd = &vmstate_rc4030_esp,
> + .qdev.reset = esp_rc4030_hard_reset,
> + .init = esp_rc4030_init1,
> + .qdev.props = (Property[]) {
> + DEFINE_PROP_TADDR("iobase", RC4030ESPState, iobase, 0x80002000),
> + DEFINE_PROP_UINT32("irq", RC4030ESPState, irq, 5),
> + DEFINE_PROP_UINT32("dma", RC4030ESPState, dma, 0),
> + DEFINE_PROP_END_OF_LIST(),
> + },
> +};
> +
> static void esp_register_devices(void)
> {
> sysbus_register_withprop(&esp_info);
> + rc4030_qdev_register(&esp_rc4030_info);
> }
>
> device_init(esp_register_devices)
> diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
> index 3c6a495..08175ab 100644
> --- a/hw/mips_jazz.c
> +++ b/hw/mips_jazz.c
> @@ -110,7 +110,6 @@ void mips_jazz_init (ram_addr_t ram_size,
>
[Qemu-devel] [Bug 586175] Re: Windows XP/2003 doesn't boot
Great solution Andreas, it worked for a Win2k image which I could only boot previously using an iso from http://www.resoo.org/docs/ntldr/files/ However, I have a w7 image that I have never managed to boot, apart from its installation cd image using virt-install 20Gb w7 image: # losetup /dev/loop0 /vm/w7.img; kpartx -a /dev/loop0 # fdisk -l /dev/loop0 Disk /dev/loop0: 21.5 GB, 21474836480 bytes 255 heads, 63 sectors/track, 2610 cylinders Units = cylinders of 16065 * 512 = 8225280 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disk identifier: 0xaf12c11f Device Boot Start End Blocks Id System /dev/loop0p1 * 1 13 1024007 HPFS/NTFS Partition 1 does not end on cylinder boundary. /dev/loop0p2 132611208670727 HPFS/NTFS Partition 2 does not end on cylinder boundary. # hexedit /dev/mapper/loop0p1 EB 52 90 4E 54 46 53 20 20 20 20 00 02 08 00 00 00 00 00 00 00 F8 00 00 3F 00 10 00 00 08 00 00 .R.NTFS.?... 0020 00 00 00 00 80 00 80 00 FF 1F 03 00 00 00 00 00 55 21 00 00 00 00 00 00 02 00 00 00 00 00 00 00 U!.. # hexedit /dev/mapper/loop0p2 EB 52 90 4E 54 46 53 20 20 20 20 00 02 08 00 00 00 00 00 00 00 F8 00 00 3F 00 10 00 00 28 03 00 .R.NTFS.?(.. 0020 00 00 00 00 80 00 80 00 FF CF 7C 02 00 00 00 00 00 00 0C 00 00 00 00 00 02 00 00 00 00 00 00 00 ..|. # kpartx -d /dev/loop0; losetup -d /dev/loop0 I changed location 0x1a to 0xFF on one or other or both partitions and it still will not boot in virt-manager. Cheers, Andy. -- Windows XP/2003 doesn't boot https://bugs.launchpad.net/bugs/586175 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Status in “qemu-kvm” package in Ubuntu: New Status in Debian GNU/Linux: New Status in Fedora: Unknown Bug description: Hello everyone, my qemu doesn't boot any Windows XP/2003 installations if I try to boot the image. If I boot the install cd first, it's boot manager counts down and triggers the boot on it's own. That's kinda stupid. I'm using libvirt, but even by a simple > qemu-kvm -drive file=image.img,media=disk,if=ide,boot=on it won't boot. Qemu hangs at the message "Booting from Hard Disk..." I'm using qemu-kvm-0.12.4 with SeaBIOS 0.5.1 on Gentoo (No-Multilib and AMD64). It's a server, that means I'm using VNC as the primary graphic output but i don't think it should be an issue.
Re: [Qemu-devel] [PATCH 3/3] savevm: prevent snapshot overwriting and generate a default name
On 07/30/10 - 10:43:01AM, Luiz Capitulino wrote: > On Fri, 30 Jul 2010 11:34:57 +0200 > Markus Armbruster wrote: > > > Miguel Di Ciurcio Filho writes: > > > > > This patch address two issues. > > > > > > 1) When savevm is run using an previously saved snapshot id or name, it > > > will > > > delete the original and create a new one, using the same id and name and > > > not > > > prompting the user of what just happened. > > > > > > This behaviour is not good, IMHO. > > > > Debatable. > > Automatically destroying previously saved data without any notice seems > a quite bad behavior to me. > > > > We add a '-f' parameter to savevm, to really force that to happen, in > > > case the > > > user really wants to. > > > > Incompatible change, looks like it'll break libvirt. Doesn't mean we > > can't do it ever, but right now may not be the best time. Perhaps after > > savevm & friends are fully functional in QMP. > > Chris, could you please check whether this impacts libvirt? Sorry for the delay here. As far as libvirt is concerned, this won't break the functionality, just change the semantics. If you only ever do snapshots through libvirt, then we can never run into this situation; libvirt prevents 2 snapshots from having the same name. Today, if you do a mixture of snapshots through the monitor and snapshots through libvirt, and you name them the same, then libvirt *could* silently override the old one. After this patch, the savevm will fail (which libvirt will gracefully handle. In any case, it's a corner case that libvirt will never intentionally put itself into, so either way is fine with me. (I also tend to think that overwriting data without any notification isn't very nice, but I also understand that this is a change in semantics) -- Chris Lalancette
[Qemu-devel] [PATCH] MIPS support for VInt and VEIC interrupt modes.
Hi,
This patch adds the final bits for supporting Vectored Interrupts
on MIPS. I also added VEIC mode, but only the logic that is part of the CPU.
VInt is the mode where the MIPS internally computes a 3 bit (0-7) vector
from the 8 hw interrupt lines.
VEIC is the mode where an external interrupt controller computes the vector
and communicates it through the IPL lines in the Cause reg.
VInt was tested by booting a linux-2.6.33 kernel, again on an out-of-tree
board. I also ran the images on the wiki to check for regressions in the
compat interrupt mode.
VEIC was very lightly tested, I dont have emulation of all the necessary
blocks to boot linux on a VEIC MIPS guest yet. I tested the CPU parts with a
couple of small synthetic irq test cases.
BTW, the way I configure VInt/VEIC is to have the board setup reconfigure
the MIPS configure bits at reset. That's why there are no changes to
target-mips/translate_init.c.
Comments?
Cheers
commit 234705e71642741ad4b8762dfb40969406d7c1ea
Author: Edgar E. Iglesias
Date: Mon Aug 2 15:50:39 2010 +0200
mips: Add support for VInt and VEIC irq modes.
Signed-off-by: Edgar E. Iglesias
diff --git a/cpu-exec.c b/cpu-exec.c
index d170566..dbdfdcc 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -448,7 +448,7 @@ int cpu_exec(CPUState *env1)
}
#elif defined(TARGET_MIPS)
if ((interrupt_request & CPU_INTERRUPT_HARD) &&
-(env->CP0_Status & env->CP0_Cause & CP0Ca_IP_mask) &&
+cpu_mips_hw_interrupts_pending(env) &&
(env->CP0_Status & (1 << CP0St_IE)) &&
!(env->CP0_Status & (1 << CP0St_EXL)) &&
!(env->CP0_Status & (1 << CP0St_ERL)) &&
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index b8e6fee..d2fe925 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -525,6 +525,29 @@ static inline void cpu_clone_regs(CPUState *env,
target_ulong newsp)
env->active_tc.gpr[2] = 0;
}
+static inline int cpu_mips_hw_interrupts_pending(CPUState *env)
+{
+int32_t pending;
+int32_t status;
+int r;
+
+pending = env->CP0_Cause & CP0Ca_IP_mask;
+status = env->CP0_Status & CP0Ca_IP_mask;
+
+if (env->CP0_Config3 & (1 << CP0C3_VEIC)) {
+/* A MIPS configured with a vectorizing external interrupt controller
+ will feed a vector into the Cause pending lines. The core treats
+ the status_lines as a vector level, not as indiviual masks. */
+r = pending > status;
+} else {
+/* A MIPS configured with compatibility or VInt (Vectored Interrupts)
+ treats the pending lines as individual interrupt lines, the status
+ lines are individual masks. */
+r = pending & status;
+}
+return r;
+}
+
#include "cpu-all.h"
/* Memory access type :
diff --git a/target-mips/helper.c b/target-mips/helper.c
index de2ed7d..bdc1e53 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -478,6 +478,33 @@ void do_interrupt (CPUState *env)
cause = 0;
if (env->CP0_Cause & (1 << CP0Ca_IV))
offset = 0x200;
+
+if (env->CP0_Config3 & ((1 << CP0C3_VInt) | (1 << CP0C3_VEIC))) {
+/* Vectored Interrupts. */
+unsigned int spacing;
+unsigned int vector;
+unsigned int pending = (env->CP0_Cause & CP0Ca_IP_mask) >> 8;
+
+/* Compute the Vector Spacing. */
+spacing = (env->CP0_IntCtl >> CP0IntCtl_VS) & ((1 << 6) - 1);
+spacing <<= 5;
+
+if (env->CP0_Config3 & (1 << CP0C3_VInt)) {
+/* For VInt mode, the MIPS computes the vector internally. */
+for (vector = 0; vector < 8; vector++) {
+if (pending & 1) {
+/* Found it. */
+break;
+}
+pending >>= 1;
+}
+} else {
+/* For VEIC mode, the external interrupt controller feeds the
+ vector throught the CP0Cause IP lines. */
+vector = pending;
+}
+offset = 0x200 + vector * spacing;
+}
goto set_EPC;
case EXCP_LTLBL:
cause = 1;
[Qemu-devel] [PATCH] msix: allow byte and word reading from mmio
It's legal that the guest reads a single byte or word from mmio.
I have an OS which reads single bytes and it works fine on real
hardware. Maybe this happens due to casting.
Signed-off-by: Bernhard Kohl
---
hw/msix.c | 20
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/hw/msix.c b/hw/msix.c
index d99403a..7dac7f7 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -100,10 +100,22 @@ static uint32_t msix_mmio_readl(void *opaque,
target_phys_addr_t addr)
return pci_get_long(page + offset);
}
-static uint32_t msix_mmio_read_unallowed(void *opaque,
target_phys_addr_t addr)
+static uint32_t msix_mmio_readw(void *opaque, target_phys_addr_t addr)
{
-fprintf(stderr, "MSI-X: only dword read is allowed!\n");
-return 0;
+PCIDevice *dev = opaque;
+unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x1;
+void *page = dev->msix_table_page;
+
+return pci_get_word(page + offset);
+}
+
+static uint32_t msix_mmio_readb(void *opaque, target_phys_addr_t addr)
+{
+PCIDevice *dev = opaque;
+unsigned int offset = addr & (MSIX_PAGE_SIZE - 1);
+void *page = dev->msix_table_page;
+
+return pci_get_byte(page + offset);
}
static uint8_t msix_pending_mask(int vector)
@@ -198,7 +210,7 @@ static CPUWriteMemoryFunc * const msix_mmio_write[] = {
};
static CPUReadMemoryFunc * const msix_mmio_read[] = {
-msix_mmio_read_unallowed, msix_mmio_read_unallowed, msix_mmio_readl
+msix_mmio_readb, msix_mmio_readw, msix_mmio_readl
};
/* Should be called from device's map method. */
--
1.7.2
[Qemu-devel] [PATCH] pckbd: support for commands 0xf0-0xff: Pulse output bit
I have a guest OS which sends the command 0xfd to the keyboard
controller during initialization. To get rid of the message
"qemu: unsupported keyboard cmd=0x%02x\n" I added support for
the pulse output bit commands.
I found the following explanation here:
http://www.win.tue.nl/~aeb/linux/kbd/scancodes-11.html#ss11.3
Command 0xf0-0xff: Pulse output bit
Bits 3-0 of the output port P2 of the keyboard controller may
be pulsed low for approximately 6 µseconds. Bits 3-0 of this
command specify the output port bits to be pulsed. 0: Bit should
be pulsed. 1: Bit should not be modified. The only useful version
of this command is Command 0xfe.
(For MCA, replace 3-0 by 1-0 in the above.)
Command 0xfe: System reset
Pulse bit 0 of the output port P2 of the keyboard controller.
This will reset the CPU.
Signed-off-by: Bernhard Kohl
---
hw/pckbd.c | 23 ---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/hw/pckbd.c b/hw/pckbd.c
index 0533b1d..6e4e406 100644
--- a/hw/pckbd.c
+++ b/hw/pckbd.c
@@ -56,7 +56,9 @@
#define KBD_CCMD_WRITE_MOUSE0xD4/* Write the following byte to
the mouse */
#define KBD_CCMD_DISABLE_A200xDD/* HP vectra only ? */
#define KBD_CCMD_ENABLE_A20 0xDF/* HP vectra only ? */
-#define KBD_CCMD_RESET0xFE
+#define KBD_CCMD_PULSE_BITS_3_0 0xF0/* Pulse bits 3-0 of the output
port P2. */
+#define KBD_CCMD_RESET 0xFE/* Pulse bit 0 of the output
port P2 = CPU reset. */
+#define KBD_CCMD_NO_OP 0xFF/* Pulse no bits of the output
port P2. */
/* Keyboard Commands */
#define KBD_CMD_SET_LEDS0xED/* Set keyboard leds */
@@ -238,6 +240,21 @@ static void kbd_write_command(void *opaque,
uint32_t addr, uint32_t val)
KBDState *s = opaque;
DPRINTF("kbd: write cmd=0x%02x\n", val);
+
+/* Bits 3-0 of the output port P2 of the keyboard controller may be
pulsed
+ * low for approximately 6 micro seconds. Bits 3-0 of the
KBD_CCMD_PULSE
+ * command specify the output port bits to be pulsed.
+ * 0: Bit should be pulsed. 1: Bit should not be modified.
+ * The only useful version of this command is pulsing bit 0,
+ * which does a CPU reset.
+ */
+if((val & KBD_CCMD_PULSE_BITS_3_0) == KBD_CCMD_PULSE_BITS_3_0) {
+if(!(val & 1))
+val = KBD_CCMD_RESET;
+else
+val = KBD_CCMD_NO_OP;
+}
+
switch(val) {
case KBD_CCMD_READ_MODE:
kbd_queue(s, s->mode, 0);
@@ -294,8 +311,8 @@ static void kbd_write_command(void *opaque, uint32_t
addr, uint32_t val)
case KBD_CCMD_RESET:
qemu_system_reset_request();
break;
-case 0xff:
-/* ignore that - I don't know what is its use */
+case KBD_CCMD_NO_OP:
+/* ignore that */
break;
default:
fprintf(stderr, "qemu: unsupported keyboard cmd=0x%02x\n", val);
--
1.7.2
[Qemu-devel] Re: Migration issues in qemu.git
On 08/02/2010 12:06 PM, Avi Kivity wrote: 2. Intermittent migration failures Likely suspect is the IDE migration fixes. A merge of the commit just before these changes passes the test, I'm now testing the commit immediately after the changes. Looks like this was a byproduct of the first problem - my disks filled up, or became extra slow during the core dump, and caused the failures. Seems to not to happen now. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: Migration issues in qemu.git
On 08/02/2010 04:12 PM, Alex Williamson wrote:
On Mon, 2010-08-02 at 12:42 +0300, Avi Kivity wrote:
On 08/02/2010 12:06 PM, Avi Kivity wrote:
I'm hitting some migration issues merging qemu.git into qemu-kvm.git:
1. Crash in mig_cancel test:
(gdb) bt
#0 0x003a91c83dbb in memcpy () from /lib64/libc.so.6
#1 0x0049c2ff in qemu_get_buffer (f=0x302d870, buf=, size1=4096) at /usr/include/bits/string3.h:52
#2 0x00409464 in ram_load (f=0x302d870, opaque=, version_id=4) at
/build/home/tlv/akivity/qemu-kvm/arch_init.c:407
#3 0x0049cb4c in qemu_loadvm_state (f=0x302d870) at
savevm.c:1708
#4 0x00494169 in process_incoming_migration (f=) at migration.c:63
#5 0x00494517 in tcp_accept_incoming_migration (opaque=) at migration-tcp.c:163
#6 0x0041b67e in main_loop_wait (nonblocking=) at /build/home/tlv/akivity/qemu-kvm/vl.c:1300
#7 0x004314e7 in kvm_main_loop () at
/build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1710
#8 0x0041c67f in main_loop (argc=,
argv=, envp=)
at /build/home/tlv/akivity/qemu-kvm/vl.c:1340
#9 main (argc=, argv=,
envp=) at /build/home/tlv/akivity/qemu-kvm/vl.c:3069
This is on the incoming side so the test completes successfully, only
leaving a core dump to fill my disks.
This appears to be
static inline void *host_from_stream_offset(QEMUFile *f,
ram_addr_t offset,
int flags)
{
static RAMBlock *block = NULL;
char id[256];
uint8_t len;
if (flags& RAM_SAVE_FLAG_CONTINUE) {
if (!block) {
fprintf(stderr, "Ack, bad migration stream!\n");
return NULL;
}
return block->host + offset;
}
with block == NULL, if my gdb-fu got a static variable in an inlined
function examined correctly.
If block == NULL, are you getting the fprintf?
Looked in qemu logs, but didn't see it. Maybe I missed it in the noise,
or maybe autotest saw SIGCHLD and closed the logging channel.
I don't see any special reason for block to be NULL on a cancelled
migration. Though perhaps the incoming stream was terminated without us
noticing, and we're migrating from some random buffer and confusing the
code?
Yeah, I don't understand that either, block == NULL should only be an
initial state, once we've seen a block it shouldn't happen. Does this
patch solve anything:
http://lists.nongnu.org/archive/html/qemu-devel/2010-07/msg01114.html
I could see this fixing it if the migration was re-attempted after the
cancel.
I'll try it manually and see.
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Re: Migration issues in qemu.git
On Mon, 2010-08-02 at 12:42 +0300, Avi Kivity wrote:
> On 08/02/2010 12:06 PM, Avi Kivity wrote:
> > I'm hitting some migration issues merging qemu.git into qemu-kvm.git:
> >
> > 1. Crash in mig_cancel test:
> >
> > (gdb) bt
> > #0 0x003a91c83dbb in memcpy () from /lib64/libc.so.6
> > #1 0x0049c2ff in qemu_get_buffer (f=0x302d870, buf= > optimized out>, size1=4096) at /usr/include/bits/string3.h:52
> > #2 0x00409464 in ram_load (f=0x302d870, opaque= > optimized out>, version_id=4) at
> > /build/home/tlv/akivity/qemu-kvm/arch_init.c:407
> > #3 0x0049cb4c in qemu_loadvm_state (f=0x302d870) at
> > savevm.c:1708
> > #4 0x00494169 in process_incoming_migration (f= > optimized out>) at migration.c:63
> > #5 0x00494517 in tcp_accept_incoming_migration (opaque= > optimized out>) at migration-tcp.c:163
> > #6 0x0041b67e in main_loop_wait (nonblocking= > out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:1300
> > #7 0x004314e7 in kvm_main_loop () at
> > /build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1710
> > #8 0x0041c67f in main_loop (argc=,
> > argv=, envp=)
> > at /build/home/tlv/akivity/qemu-kvm/vl.c:1340
> > #9 main (argc=, argv=,
> > envp=) at /build/home/tlv/akivity/qemu-kvm/vl.c:3069
> >
> > This is on the incoming side so the test completes successfully, only
> > leaving a core dump to fill my disks.
>
>
> This appears to be
>
> > static inline void *host_from_stream_offset(QEMUFile *f,
> > ram_addr_t offset,
> > int flags)
> > {
> > static RAMBlock *block = NULL;
> > char id[256];
> > uint8_t len;
> >
> > if (flags & RAM_SAVE_FLAG_CONTINUE) {
> > if (!block) {
> > fprintf(stderr, "Ack, bad migration stream!\n");
> > return NULL;
> > }
> >
> > return block->host + offset;
> > }
>
> with block == NULL, if my gdb-fu got a static variable in an inlined
> function examined correctly.
If block == NULL, are you getting the fprintf?
> I don't see any special reason for block to be NULL on a cancelled
> migration. Though perhaps the incoming stream was terminated without us
> noticing, and we're migrating from some random buffer and confusing the
> code?
Yeah, I don't understand that either, block == NULL should only be an
initial state, once we've seen a block it shouldn't happen. Does this
patch solve anything:
http://lists.nongnu.org/archive/html/qemu-devel/2010-07/msg01114.html
I could see this fixing it if the migration was re-attempted after the
cancel.
Alex
[Qemu-devel] Still unable to complete Windows XP install
Even with qemu-kvm 0.12.5, at the end of the first stage of Windows XP installation, the virtual machine reboots and the console shows: "A disk read error occured Press Ctrl+Alt+Del to restart" when using libvirt (-drive + -device options). -- Laurent Léonard
Re: [Qemu-devel] [PATCH 2/3] cleanup: del_existing_snapshots() must return the upstream error code
Miguel Di Ciurcio Filho writes: > On Fri, Jul 30, 2010 at 6:45 AM, Markus Armbruster wrote: >> Why? >> >> I figure the next patch wants it, but if that's the reason, the commit >> message should state it. >> > > To better identify what happened and where, IMHO. To let the *next* patch do a better job. That's not obvious from this patch, so better state it.
Re: [Qemu-devel] [Bug 611646] [NEW] isa bus not working
Isaku Yamahata writes:
> Sorry for that.
> Does the attached patch fix it?
As far as I can see, yes.
I'd prefer:
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 812ddfd..34c65d5 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -103,6 +103,7 @@ static void pc_init1(ram_addr_t ram_size,
pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, isa_irq, ram_size);
} else {
pci_bus = NULL;
+i440fx_state = NULL;
isa_bus_new(NULL);
}
isa_bus_irqs(isa_irq);
Matter of taste, so you get to pick.
[Qemu-devel] Re: Migration issues in qemu.git
On 08/02/2010 12:06 PM, Avi Kivity wrote:
I'm hitting some migration issues merging qemu.git into qemu-kvm.git:
1. Crash in mig_cancel test:
(gdb) bt
#0 0x003a91c83dbb in memcpy () from /lib64/libc.so.6
#1 0x0049c2ff in qemu_get_buffer (f=0x302d870, buf=optimized out>, size1=4096) at /usr/include/bits/string3.h:52
#2 0x00409464 in ram_load (f=0x302d870, opaque=optimized out>, version_id=4) at
/build/home/tlv/akivity/qemu-kvm/arch_init.c:407
#3 0x0049cb4c in qemu_loadvm_state (f=0x302d870) at
savevm.c:1708
#4 0x00494169 in process_incoming_migration (f=optimized out>) at migration.c:63
#5 0x00494517 in tcp_accept_incoming_migration (opaque=optimized out>) at migration-tcp.c:163
#6 0x0041b67e in main_loop_wait (nonblocking=out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:1300
#7 0x004314e7 in kvm_main_loop () at
/build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1710
#8 0x0041c67f in main_loop (argc=,
argv=, envp=)
at /build/home/tlv/akivity/qemu-kvm/vl.c:1340
#9 main (argc=, argv=,
envp=) at /build/home/tlv/akivity/qemu-kvm/vl.c:3069
This is on the incoming side so the test completes successfully, only
leaving a core dump to fill my disks.
This appears to be
static inline void *host_from_stream_offset(QEMUFile *f,
ram_addr_t offset,
int flags)
{
static RAMBlock *block = NULL;
char id[256];
uint8_t len;
if (flags & RAM_SAVE_FLAG_CONTINUE) {
if (!block) {
fprintf(stderr, "Ack, bad migration stream!\n");
return NULL;
}
return block->host + offset;
}
with block == NULL, if my gdb-fu got a static variable in an inlined
function examined correctly.
I don't see any special reason for block to be NULL on a cancelled
migration. Though perhaps the incoming stream was terminated without us
noticing, and we're migrating from some random buffer and confusing the
code?
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Migration issues in qemu.git
I'm hitting some migration issues merging qemu.git into qemu-kvm.git: 1. Crash in mig_cancel test: (gdb) bt #0 0x003a91c83dbb in memcpy () from /lib64/libc.so.6 #1 0x0049c2ff in qemu_get_buffer (f=0x302d870, buf=optimized out>, size1=4096) at /usr/include/bits/string3.h:52 #2 0x00409464 in ram_load (f=0x302d870, opaque=out>, version_id=4) at /build/home/tlv/akivity/qemu-kvm/arch_init.c:407 #3 0x0049cb4c in qemu_loadvm_state (f=0x302d870) at savevm.c:1708 #4 0x00494169 in process_incoming_migration (f=out>) at migration.c:63 #5 0x00494517 in tcp_accept_incoming_migration (opaque=optimized out>) at migration-tcp.c:163 #6 0x0041b67e in main_loop_wait (nonblocking=out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:1300 #7 0x004314e7 in kvm_main_loop () at /build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1710 #8 0x0041c67f in main_loop (argc=, argv=, envp=) at /build/home/tlv/akivity/qemu-kvm/vl.c:1340 #9 main (argc=, argv=, envp=) at /build/home/tlv/akivity/qemu-kvm/vl.c:3069 This is on the incoming side so the test completes successfully, only leaving a core dump to fill my disks. 2. Intermittent migration failures Likely suspect is the IDE migration fixes. A merge of the commit just before these changes passes the test, I'm now testing the commit immediately after the changes. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole
On (Mon) Aug 02 2010 [04:33:49], Alon Levy wrote: > Hi, > > This patch adds three CHR_IOCTLs and uses them in virtserial devices, to be > used > by a chardev backend, such as a spice vm channel (spice is a vdi solution). > > Basically virtio-serial provides three driver initiated events for guest > open of > a device, guest close, and guest ready (driver port init complete) that > before this > patch are not exposed to the chardev backend. > > With the spicevmc backend this is used like this: > qemu -chardev spicevmc,id=vdiport,name=vdiport -device > virtserialport,chardev=vdiport,name=com.redhat.spice.0 > > I'd appreciate any feedback if this seems the right way to accomplish this, > and > for the numbers I grabbed. Looks fine to me, maybe you don't need the notifications for virtio-console ports, only the virtio-serial ports could be of interest, but I'm fine with both notifying as well. Also, this description could be made part of the patch description itself. Amit
Re: [Qemu-devel] [Bug 611646] [NEW] isa bus not working
Sorry for that. Does the attached patch fix it? >From 20b13fa4a2c5e755346f7a91d44d23dd781a87fa Mon Sep 17 00:00:00 2001 Message-Id: <20b13fa4a2c5e755346f7a91d44d23dd781a87fa.1280738898.git.yamah...@valinux.co.jp> In-Reply-To: References: From: Isaku Yamahata Date: Mon, 2 Aug 2010 17:47:07 +0900 Subject: [PATCH] isapc: fix segfault. This patch fixes the following segfault introduced by f885f1eaa8711c06033ceb1599e3750fb37c306f i440fx_state in pc_init1() isn't initialized. > Core was generated by `./i386-softmmu/qemu -M isapc'. > Program terminated with signal 11, Segmentation fault. > [New process 19686] > at > /home/yamahata/xen/iommu/qemu/git/mkpatch/qemu-isapc-fix-0/hw/piix_pci.c:136 > (gdb) where > at > /home/yamahata/xen/iommu/qemu/git/mkpatch/qemu-isapc-fix-0/hw/piix_pci.c:136 > boot_device=0x7fffe1f5b040 "cad", kernel_filename=0x0, > kernel_cmdline=0x6469bf "", initrd_filename=0x0, > cpu_model=0x654d10 "486", pci_enabled=0) > at > /home/yamahata/xen/iommu/qemu/git/mkpatch/qemu-isapc-fix-0/hw/pc_piix.c:178 > boot_device=0x7fffe1f5b040 "cad", kernel_filename=0x0, > kernel_cmdline=0x6469bf "", initrd_filename=0x0, cpu_model=0x654d10 "486") > at > /home/yamahata/xen/iommu/qemu/git/mkpatch/qemu-isapc-fix-0/hw/pc_piix.c:207 > envp=0x7fffe1f5b188) > at /home/yamahata/xen/iommu/qemu/git/mkpatch/qemu-isapc-fix-0/vl.c:2871 Signed-off-by: Isaku Yamahata --- hw/pc_piix.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 812ddfd..634e8e6 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -69,7 +69,7 @@ static void pc_init1(ram_addr_t ram_size, int i; ram_addr_t below_4g_mem_size, above_4g_mem_size; PCIBus *pci_bus; -PCII440FXState *i440fx_state; +PCII440FXState *i440fx_state = NULL; int piix3_devfn = -1; qemu_irq *cpu_irq; qemu_irq *isa_irq; -- 1.7.1.1 On Mon, Aug 02, 2010 at 10:22:43AM +0200, Markus Armbruster wrote: > Victor Shkamerda <611...@bugs.launchpad.net> writes: > > > Public bug reported: > > > > isa bus emulation not working anymore. > > > > Try running "qemu -M isapc". It will crash with segmentation fault. > > > > This is a qemu HEAD from git on Fedora linux. > > > > ** Affects: qemu > > Importance: Undecided > > Status: New > > git bisect points to > > commit f885f1eaa8711c06033ceb1599e3750fb37c306f > Author: Isaku Yamahata > Date: Fri May 14 16:29:04 2010 +0900 > > pc, i440fx: Make smm enable/disable function i440fx independent. > > make cpu_smm_update() generic to be independent on i440fx by > registering a callback. > > Signed-off-by: Isaku Yamahata > Acked-by: Gerd Hoffmann > Signed-off-by: Blue Swirl > -- yamahata
[Qemu-devel] Re: [PATCH] PPC4xx: don't unregister RAM at reset
On 30.07.2010, at 03:48, Hollis Blanchard wrote: > The PowerPC 4xx SDRAM controller emulation unregisters RAM in its reset > callback. However, qemu_system_reset() is now called at initialization > time, so RAM is unregistered before starting the guest. So the registration should be moved to reset now, no? How is the reset different from boot? How did a reset work before? Alex
Re: [Qemu-devel] [PATCH] qemu-option: Include name of invalid parameter in error message
Stefan Weil writes:
> All other error messages in qemu-option.c display the name
> of the invalid parameter. This seems to be reasonable for
> invalid identifiers, too. Without it, a debugger is needed
> to find the name.
>
> Cc: Markus Armbruster
> Cc: Anthony Liguori
> Signed-off-by: Stefan Weil
> ---
> qemu-option.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-option.c b/qemu-option.c
> index 1f8f41a..ccea267 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -694,7 +694,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char
> *id, int fail_if_exist
>
> if (id) {
> if (!id_wellformed(id)) {
> -qerror_report(QERR_INVALID_PARAMETER_VALUE, "id", "an
> identifier");
> +qerror_report(QERR_INVALID_PARAMETER_VALUE, id, "an identifier");
> error_printf_unless_qmp("Identifiers consist of letters, digits,
> '-', '.', '_', starting with a letter.\n");
> return NULL;
> }
No.
QERR_INVALID_PARAMETER_VALUE's first argument is the parameter *name*,
not the parameter *value*.
In this case, the parameter name is "id". The variable id contains the
parameter value.
If you need a debugger to find the offending id=, then location
information is lacking. Could you give an example where it's hard to
find the offending parameter?
Here's an example where it's easy to find:
$ qemu-system-x86_64 -device e1000,id=.
qemu-system-x86_64: -device e1000,id=.: Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a
letter.
[Qemu-devel] RFC adding ioctl's to virtserial/virtconsole
Hi,
This patch adds three CHR_IOCTLs and uses them in virtserial devices, to be
used
by a chardev backend, such as a spice vm channel (spice is a vdi solution).
Basically virtio-serial provides three driver initiated events for guest open
of
a device, guest close, and guest ready (driver port init complete) that before
this
patch are not exposed to the chardev backend.
With the spicevmc backend this is used like this:
qemu -chardev spicevmc,id=vdiport,name=vdiport -device
virtserialport,chardev=vdiport,name=com.redhat.spice.0
I'd appreciate any feedback if this seems the right way to accomplish this, and
for the numbers I grabbed.
Alon
-- commit message
>From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:00 2001
From: Alon Levy
Date: Mon, 2 Aug 2010 11:22:58 +0300
Subject: [PATCH] virtio-console: add IOCTL's for guest_{ready,open,close}
Add three IOCTL corresponding to the three control events of:
guest_ready -> CHR_IOCTL_VIRT_SERIAL_READY
guest_open -> CHR_IOCTL_VIRT_SERIAL_OPEN
guest_close -> CHR_IOCTL_VIRT_SERIAL_CLOSE
Can be used by a matching backend.
---
hw/virtio-console.c | 33 +
qemu-char.h |4
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index caea11f..4c3686d 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -58,6 +58,33 @@ static void chr_event(void *opaque, int event)
}
}
+static void virtconsole_guest_open(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_OPEN, NULL);
+}
+}
+
+static void virtconsole_guest_close(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_CLOSE, NULL);
+}
+}
+
+static void virtconsole_guest_ready(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+if (vcon->chr) {
+qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_READY, NULL);
+}
+}
+
/* Virtio Console Ports */
static int virtconsole_initfn(VirtIOSerialDevice *dev)
{
@@ -94,6 +121,9 @@ static VirtIOSerialPortInfo virtconsole_info = {
.qdev.size = sizeof(VirtConsole),
.init = virtconsole_initfn,
.exit = virtconsole_exitfn,
+.guest_open= virtconsole_guest_open,
+.guest_close = virtconsole_guest_close,
+.guest_ready = virtconsole_guest_ready,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT8("is_console", VirtConsole, port.is_console, 1),
DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
@@ -130,6 +160,9 @@ static VirtIOSerialPortInfo virtserialport_info = {
.qdev.size = sizeof(VirtConsole),
.init = virtserialport_initfn,
.exit = virtconsole_exitfn,
+.guest_open= virtconsole_guest_open,
+.guest_close = virtconsole_guest_close,
+.guest_ready = virtconsole_guest_ready,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR("chardev", VirtConsole, chr),
diff --git a/qemu-char.h b/qemu-char.h
index e3a0783..1df53ae 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -41,6 +41,10 @@ typedef struct {
#define CHR_IOCTL_SERIAL_SET_TIOCM 13
#define CHR_IOCTL_SERIAL_GET_TIOCM 14
+#define CHR_IOCTL_VIRT_SERIAL_OPEN 15
+#define CHR_IOCTL_VIRT_SERIAL_CLOSE 16
+#define CHR_IOCTL_VIRT_SERIAL_READY 17
+
#define CHR_TIOCM_CTS 0x020
#define CHR_TIOCM_CAR 0x040
#define CHR_TIOCM_DSR 0x100
--
1.7.2
Re: [Qemu-devel] [Bug 611646] [NEW] isa bus not working
Victor Shkamerda <611...@bugs.launchpad.net> writes: > Public bug reported: > > isa bus emulation not working anymore. > > Try running "qemu -M isapc". It will crash with segmentation fault. > > This is a qemu HEAD from git on Fedora linux. > > ** Affects: qemu > Importance: Undecided > Status: New git bisect points to commit f885f1eaa8711c06033ceb1599e3750fb37c306f Author: Isaku Yamahata Date: Fri May 14 16:29:04 2010 +0900 pc, i440fx: Make smm enable/disable function i440fx independent. make cpu_smm_update() generic to be independent on i440fx by registering a callback. Signed-off-by: Isaku Yamahata Acked-by: Gerd Hoffmann Signed-off-by: Blue Swirl
[Qemu-devel] [Bug 612452] [NEW] Problems with the number of serial ports for more than two
Public bug reported: qemu --version QEMU emulator version 0.13.50, Copyright (c) 2003-2008 Fabrice Bellard Command line: qemu -serial null -serial null -serial file:test1 hd.img Error: isa irq 4 already assigned echo $? 1 ** Affects: qemu Importance: Undecided Status: New -- Problems with the number of serial ports for more than two https://bugs.launchpad.net/bugs/612452 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: qemu --version QEMU emulator version 0.13.50, Copyright (c) 2003-2008 Fabrice Bellard Command line: qemu -serial null -serial null -serial file:test1 hd.img Error: isa irq 4 already assigned echo $? 1
Re: [Qemu-devel] [Bug 611640] [NEW] snapshot mode is broken for raw images
Victor Shkamerda <611...@bugs.launchpad.net> writes: > Public bug reported: > > The snapshot mode is not working if you use raw format for image instead > of qcow2. Create a raw disk image by running "dd of=xxx.img bs=1 count=0 > seek=8G". Then run "qemu -snapshot xxx.img". In monitor console run > "info block". There should be qcow2 temporary image backed by the raw > image. Now run "commit ide0-hd0" and after that again "info block". You > should see now that there is no more image, so any operations on it will > fail form now on. This is for latest HEAD in git. > > ** Affects: qemu > Importance: Undecided > Status: New git bisect points to commit c33491978c5c4c746cc0272d5f2df50aa14d1f02 Author: Kevin Wolf Date: Thu May 6 16:34:56 2010 +0200 block: Fix bdrv_commit When reopening the image, don't guess the driver, but use the same driver as was used before. This is important if the format=... option was used for that image.
[Qemu-devel] Re: [TUHS & QEMU] Making progress with old DG/UX virtualization. Need advice.
Thanks Natalia, I'll start by answering the insultive part of your answer, as my ego will not let me go on if I don't: I am not "begging on all the internet", I am simply seeking solutions, help and advice, and making sure to update whoever is interested in the progress I am doing. Also, I wish to thank you for your insight and well detailed answer. Finally I got an explanation as to _why_ solution A will not be as good as solution B. That is what I call a winning argument, and I thank you for that. I already have people searching for Adaptec docs and programmers for the creation of the driver, err, emulated device. Take care, Adam On Mon, Aug 2, 2010 at 9:11 AM, Natalia Portillo wrote: > Hi, > > I've read all your posts in the QEMU mailing list and the TUHS one and I'm > answering to both lists in a hope my mail enlights you and any other curious. > > First of all, old UNIX systems (and I put my hand on the fire for DG/UX > also), use a monolithic linked at setup/later time kernel. > That is, even if you get a driver (IDE, virtio, whatsoever), the > configuration files, the kernel, the ramdisk, everything that lets your > system boot, MUST HAVE BEEN BOOT from the AIC controller, the driver is > hardcoded, no way to change it. > > If you have extensive knowledge of what files a driver setup modifies on > DG/UX specifically (knowledge from other UNIX, forget it, they are as > different as Porsche and Ferrari motors), you can always get a new kernel > with the drivers you need to make it boot and manually put them in your image. > > In the case, you meet this requirements, and, you do it, you can then achieve > to other problems. The DG/UX workstations are x86 machines, but nothing > swears they are PC compatible machines, and they can have a different memory > map for some critical device, or include critical devices never found in a PC > (like an Intel Macintosh does for example). Just booting from a BIOS doesn't > make the machines be the same (PowerPC Macintosh, IBM POWER workstations, > Genesi Pegasos, are machines that boot OpenFirmware with heavily different > configurations, devices and memory maps). > > Also, you are assuming IDE is available in DG/UX just because the controller > is present in the hardware. That hardware was also used for Windows NT. IDE > support can be JUST FOR Windows, and the DG/UX manufacturer just decided to > not include an IDE driver in the kernel (happened in AIX for PCs until last > version of all, only SCSI was supported, being a hugely strange controller in > PC worlds). > > In the case you opt for making a driver (adding IDE, virtio, or other SCSI > support) for the DG/UX need to say you need, low level knowledge of the > hardware, low level knowledge of the operating system, a working machine (for > sure, with the hardware available), a debug machine (almost sure also), C and > maybe assembler knowledge. In a scale of 10, this puts the difficulty in 8 > for most of programmers, and surely if you were one you stacked with the > first option everyone gave you (see next sentence). > > The easiest way, and the one that people answered you already in QEMU's > mailing list (in a scale of 10 the difficulty is 6 or even 5), is creating an > emulated device (that's the correct term, not "driver") for an emulator, like > QEMU, Bochs, VirtualBox (forget this option for VMWare, VirtualPC or > Parallels) that adds the AIC SCSI controller you exactly need. > > Why is this easiest? You don't need any DG/UX working system, you don't need > to know how DG/UX works, you don't need to compile a kernel, copy it to your > image. > > You just take the Adaptec's documentation, and start coding, making a SCSI > emulated controller, and testing it with systems you can always reinstall, > debug, and check, until they fully work (Windows, Linux, BSD, take your > choice). > > And then, you just polish it until your DG/UX boots, or finds the memory map > as a mess it doesnt like. > > Finally, please stop begging on all the internet, spend that time coding the > driver or getting the money to pay a programmer that will do. > > Sincerely yours, > Natalia Portillo > Claunia.com CEO > QEMU's Official OS Support List maintainer
