Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Dustin Kirkland
On Wed, 2009-11-04 at 18:52 -0600, Anthony Liguori wrote:
> I'd rather make the "default" network configurable via a global 
> configuration file.  That way, if a distribution knew that it had a 
> bridge setup for its users, it could make -net bridge the default.

Fair enough.

:-Dustin


signature.asc
Description: This is a digitally signed message part


Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Anthony Liguori

Dustin Kirkland wrote:

We address this problem by introducing a new network backend: -net bridge.  This
backend is less flexible than -net tap because it relies on a helper with
elevated privileges to do the heavy lifting of allocating and attaching a tap
device to a bridge.  We use a special purpose helper because we don't want
to elevate the privileges of more generic tools like brctl.

From a user perspective, to use bridged networking with a guest, you simply use:

  qemu -hda linux.img -net bridge -net nic



I know that this patch is less than a day old and untested, but would it
be reasonable to make this the "default" network configuration at some
point in the future?  This certainly seems to be what I want 99% of the
time when I launch qemu or kvm by hand from the command line.
  


I'd rather make the "default" network configurable via a global 
configuration file.  That way, if a distribution knew that it had a 
bridge setup for its users, it could make -net bridge the default.


Regards,

Anthony Liguori




[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Dustin Kirkland
On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote:
> This series solves a problem that I've been struggling with for a few years 
> now.
> One of the best things about qemu is that it's possible to run guests as an
> unprivileged user to improve security.  However, if you want to have your 
> guests
> communicate with the outside world, you're pretty much forced to run qemu as
> root.
> 
> At least with KVM support, this is probably the most common use case which 
> means
> that most of our users are running qemu as root.  That's terrible.

Ack.

> We address this problem by introducing a new network backend: -net bridge.  
> This
> backend is less flexible than -net tap because it relies on a helper with
> elevated privileges to do the heavy lifting of allocating and attaching a tap
> device to a bridge.  We use a special purpose helper because we don't want
> to elevate the privileges of more generic tools like brctl.
> 
> From a user perspective, to use bridged networking with a guest, you simply 
> use:
> 
>   qemu -hda linux.img -net bridge -net nic

I know that this patch is less than a day old and untested, but would it
be reasonable to make this the "default" network configuration at some
point in the future?  This certainly seems to be what I want 99% of the
time when I launch qemu or kvm by hand from the command line.

> And assuming a bridge is defined named qemubr0 and the administrator has setup
> permissions accordingly, it will Just Work.  My hope is that distributions 
> will
> do this work as part of the qemu packaging process such that for most users,
> the out-of-the-box experience will also Just Work.

Also, ack.  I'll handle the Ubuntu packaging to enable this support in
Lucid by the time qemu-0.12-rc1 is available.  As Alexander mentions,
there's a bit more complexity we'll need to account for (wifi, network
manager, multiple nic's).

:-Dustin


signature.asc
Description: This is a digitally signed message part


Re: [Qemu-devel] Inquiry:Solaris 8 installation on QEMU

2009-11-04 Thread Artyom Tarasenko
>> The kernels of Solaris 8 & 9 can be boot too, but then they flood
>> about spurious irq 10. It seems that the earlier Solaris versions are
>> also suffering from spurious interrupts, because the boot process
>> takes very long: ~7 hours on e8...@2.66ghz .
>
> A bug in the system timer implementation?

Or in interrupt handling in general. The interrupts seem to be really
spurious, not triggered by the timer. I think I fixed one bug in
slavio today, but I need more reading slavio docs. With the fix
solaris 9 seems to boot as well. Although it still complains about
spurious interrupts, but with the fix it happens once a second and not
hundreds times a second as before.

The performance is not affected, so either the fix is incomplete, or I
took a wrong trail




Re: [Qemu-devel] [PATCH] don't call reset functions on cpu initialization\

2009-11-04 Thread Glauber Costa
On Wed, Nov 04, 2009 at 02:51:08PM -0200, Marcelo Tosatti wrote:
> On Tue, Nov 03, 2009 at 05:50:05PM -0200, Glauber Costa wrote:
> > There is absolutely no need to call reset functions when initializing
> > devices. Since we are already registering them, calling qemu_system_reset()
> > should suffice. Actually, it is what happens when we reboot the machine,
> > and using the same process instead of a special case semantics will even
> > allow us to find bugs easier.
> > 
> > Furthermore, the fact that we initialize things like the cpu quite early,
> > leads to the need to introduce synchronization stuff like qemu_system_cond.
> > This patch removes it entirely. All we need to do is call 
> > qemu_system_reset()
> > only when we're already sure the system is up and running
> > 
> > I tested it with qemu (with and without io-thread) and qemu-kvm, and it
> > seems to be doing okay - although qemu-kvm uses a slightly different patch.
> > 
> > Signed-off-by: Glauber Costa 
> > ---
> >  hw/apic.c|1 -
> >  hw/e1000.c   |1 -
> >  hw/hpet.c|1 -
> >  hw/i8254.c   |2 --
> >  hw/ide/piix.c|1 -
> >  hw/piix4.c   |1 -
> >  hw/piix_pci.c|1 -
> >  hw/rtl8139.c |2 +-
> >  hw/serial.c  |1 -
> >  hw/usb-ohci.c|1 -
> >  hw/usb-uhci.c|1 -
> >  hw/vga.c |1 -
> >  target-i386/helper.c |1 -
> >  vl.c |   15 +--
> >  14 files changed, 2 insertions(+), 28 deletions(-)
> > 
> > diff --git a/hw/apic.c b/hw/apic.c
> > index c89008e..87e7dc0 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -981,7 +981,6 @@ int apic_init(CPUState *env)
> >  s->id = env->cpuid_apic_id;
> >  s->cpu_env = env;
> >  
> > -apic_reset(s);
> >  msix_supported = 1;
> >  
> >  /* XXX: mapping more APICs at the same memory location */
> > diff --git a/hw/e1000.c b/hw/e1000.c
> > index 028afd1..8fb299d 100644
> > --- a/hw/e1000.c
> > +++ b/hw/e1000.c
> > @@ -1113,7 +1113,6 @@ static int pci_e1000_init(PCIDevice *pci_dev)
> >  qemu_format_nic_info_str(d->vc, macaddr);
> >  
> >  vmstate_register(-1, &vmstate_e1000, d);
> > -e1000_reset(d);
> >  
> >  if (!pci_dev->qdev.hotplugged) {
> >  static int loaded = 0;
> > diff --git a/hw/hpet.c b/hw/hpet.c
> > index 64163bd..6f39711 100644
> > --- a/hw/hpet.c
> > +++ b/hw/hpet.c
> > @@ -577,7 +577,6 @@ void hpet_init(qemu_irq *irq) {
> >  HPETTimer *timer = &s->timer[i];
> >  timer->qemu_timer = qemu_new_timer(vm_clock, hpet_timer, timer);
> >  }
> > -hpet_reset(s);
> >  vmstate_register(-1, &vmstate_hpet, s);
> >  qemu_register_reset(hpet_reset, s);
> >  /* HPET Area */
> > diff --git a/hw/i8254.c b/hw/i8254.c
> > index 5c49e6e..faaa884 100644
> > --- a/hw/i8254.c
> > +++ b/hw/i8254.c
> > @@ -513,7 +513,5 @@ PITState *pit_init(int base, qemu_irq irq)
> >  register_ioport_write(base, 4, 1, pit_ioport_write, pit);
> >  register_ioport_read(base, 3, 1, pit_ioport_read, pit);
> >  
> > -pit_reset(pit);
> > -
> >  return pit;
> >  }
> > diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> > index a17bf59..60b37a3 100644
> > --- a/hw/ide/piix.c
> > +++ b/hw/ide/piix.c
> > @@ -120,7 +120,6 @@ static int pci_piix_ide_initfn(PCIIDEState *d)
> >  pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
> >  
> >  qemu_register_reset(piix3_reset, d);
> > -piix3_reset(d);
> >  
> >  pci_register_bar(&d->dev, 4, 0x10, PCI_ADDRESS_SPACE_IO, bmdma_map);
> >  
> > diff --git a/hw/piix4.c b/hw/piix4.c
> > index a6aea15..f75951b 100644
> > --- a/hw/piix4.c
> > +++ b/hw/piix4.c
> > @@ -97,7 +97,6 @@ static int piix4_initfn(PCIDevice *d)
> >  PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // 
> > header_type = PCI_multifunction, generic
> >  
> >  piix4_dev = d;
> > -piix4_reset(d);
> >  qemu_register_reset(piix4_reset, d);
> >  return 0;
> >  }
> > diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> > index ed036fe..20d834f 100644
> > --- a/hw/piix_pci.c
> > +++ b/hw/piix_pci.c
> > @@ -341,7 +341,6 @@ static int piix3_initfn(PCIDevice *dev)
> >  pci_conf[PCI_HEADER_TYPE] =
> >  PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // 
> > header_type = PCI_multifunction, generic
> >  
> > -piix3_reset(d);
> >  qemu_register_reset(piix3_reset, d);
> >  return 0;
> >  }
> > diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> > index d26df48..27cc618 100644
> > --- a/hw/rtl8139.c
> > +++ b/hw/rtl8139.c
> > @@ -3331,7 +3331,7 @@ static int pci_rtl8139_init(PCIDevice *dev)
> > PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_map);
> >  
> >  qemu_macaddr_default_if_unset(&s->conf.macaddr);
> > -rtl8139_reset(&s->dev.qdev);
> > +
> >  s->vc = qemu_new_vlan_client(NET_CLIENT_TYPE_NIC,
> >   s->conf.vlan, s->conf.peer,
> >  

Re: [Qemu-devel] [PATCH] Fix a parallel build failure.

2009-11-04 Thread Daniel Jacobowitz
On Wed, Nov 04, 2009 at 09:48:22PM +0100, Stefan Weil wrote:
> Maybe a mix of your patch and my patch
> (http://patchwork.ozlabs.org/patch/37446/)
> would be the best fix for this problem.
> 
> It should also be possible to apply both patches.

Thanks, I didn't find your patch.  I don't have any preference which
is applied, although I'm always in favor of eliminating unnecessary
recursive invocations.

-- 
Daniel Jacobowitz
CodeSourcery




Re: [Qemu-devel] [PATCH] Fix a parallel build failure.

2009-11-04 Thread Stefan Weil
Daniel Jacobowitz schrieb:
> From: Daniel Jacobowitz 
>
> With enough parallelism, make will run all the dependencies of
> build-all at the same time:
>
> build-all: config-host.h config-all-devices.h $(DOCS) $(TOOLS)
>
> So some of the $(TOOLS) will build before config-host.h is finished.
> The object files need to depend on it explicitly.  Subdirectories are
> OK since they are started from the body of build-all, not its
> dependencies.
>
> Signed-off-by: Daniel Jacobowitz 
> ---
>  Makefile  |2 ++
>  rules.mak |2 +-
>  2 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c783aa4..ed9a420 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,5 +1,7 @@
>  # Makefile for QEMU.
>  
> +GENERATED_HEADERS = config-host.h config-all-devices.h
> +
>  ifneq ($(wildcard config-host.mak),)
>  # Put the all: rule here so that config-host.mak can contain dependencies.
>  all: build-all
> diff --git a/rules.mak b/rules.mak
> index 5d7e8bb..4eb1f90 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -13,7 +13,7 @@ MAKEFLAGS += -rR
>  
>  QEMU_CFLAGS += -MMD -MP -MT $@
>  
> -%.o: %.c
> +%.o: %.c $(GENERATED_HEADERS)
>   $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<,"  CC   
>  $(TARGET_DIR)$@")
>  
>  %.o: %.S
>   

Maybe a mix of your patch and my patch
(http://patchwork.ozlabs.org/patch/37446/)
would be the best fix for this problem.

It should also be possible to apply both patches.

Regards
Stefan





Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Anthony Liguori

Michael S. Tsirkin wrote:
Well it doesn't really help with the issue of privileges which is what  
this series is really about.


Regards,

Anthony Liguori



I note that by default you grant all users all access.
If you do that, just give them net cap admin already?
  


By default, I give no users any access.

qemu-bridge-helper carries cap_net_admin but it doesn't do everything 
cap_net_admin does.  Since an administrator has to set that capability, 
the admin is going to make it owned by root so that an unprivileged user 
cannot change it.  Modulo bugs, it's a very restricted subset of 
cap_net_admin.


In order for a user to be able to get a tap device connected to a 
bridge, the following things must be true:


1) the user must have execute privileges for qemu-bridge-helper
2) the user must have read/write access to /dev/net/tun
3) there must be an /etc/qemu/bridge.conf that is readable by the user
4) the config must have an explicit rule allowing access to the required 
bridge device


So the user is very restricted in what they can do and they must be 
granted these permissions explicitly by an administrator.  By using 
multiple bridge.conf files, an administrator can also create policies 
based on filesystem permissions allowing certain user/groups to access 
only certain bridges.


With raw, qemu must carry cap_net_raw.   That is definitely not safe for 
an untrusted user.  Allowing an untrusted user to connect a VM to a 
bridged physical network, on the other hand, seems to be a rather safe 
thing to do as long as there are strongly ways to control which bridges 
they can connect to.


Regards,

Anthony Liguori




[Qemu-devel] [PATCH] Fix a parallel build failure.

2009-11-04 Thread Daniel Jacobowitz
From: Daniel Jacobowitz 

With enough parallelism, make will run all the dependencies of
build-all at the same time:

build-all: config-host.h config-all-devices.h $(DOCS) $(TOOLS)

So some of the $(TOOLS) will build before config-host.h is finished.
The object files need to depend on it explicitly.  Subdirectories are
OK since they are started from the body of build-all, not its
dependencies.

Signed-off-by: Daniel Jacobowitz 
---
 Makefile  |2 ++
 rules.mak |2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index c783aa4..ed9a420 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,7 @@
 # Makefile for QEMU.
 
+GENERATED_HEADERS = config-host.h config-all-devices.h
+
 ifneq ($(wildcard config-host.mak),)
 # Put the all: rule here so that config-host.mak can contain dependencies.
 all: build-all
diff --git a/rules.mak b/rules.mak
index 5d7e8bb..4eb1f90 100644
--- a/rules.mak
+++ b/rules.mak
@@ -13,7 +13,7 @@ MAKEFLAGS += -rR
 
 QEMU_CFLAGS += -MMD -MP -MT $@
 
-%.o: %.c
+%.o: %.c $(GENERATED_HEADERS)
$(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<,"  CC   
 $(TARGET_DIR)$@")
 
 %.o: %.S
-- 
1.6.5.2





Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Michael S. Tsirkin
On Wed, Nov 04, 2009 at 01:48:01PM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote:
>>   
>>> This series solves a problem that I've been struggling with for a few years 
>>> now.
>>> One of the best things about qemu is that it's possible to run guests as an
>>> unprivileged user to improve security.  However, if you want to have your 
>>> guests
>>> communicate with the outside world, you're pretty much forced to run qemu as
>>> root.
>>>
>>> At least with KVM support, this is probably the most common use case which 
>>> means
>>> that most of our users are running qemu as root.  That's terrible.
>>>
>>> We address this problem by introducing a new network backend: -net bridge.  
>>> This
>>> backend is less flexible than -net tap because it relies on a helper with
>>> elevated privileges to do the heavy lifting of allocating and attaching a 
>>> tap
>>> device to a bridge.  We use a special purpose helper because we don't want
>>> to elevate the privileges of more generic tools like brctl.
>>>
>>> >From a user perspective, to use bridged networking with a guest, you 
>>> >simply use:
>>>
>>>   qemu -hda linux.img -net bridge -net nic
>>>
>>> And assuming a bridge is defined named qemubr0 and the administrator has 
>>> setup
>>> permissions accordingly, it will Just Work.  My hope is that distributions 
>>> will
>>> do this work as part of the qemu packaging process such that for most users,
>>> the out-of-the-box experience will also Just Work.
>>>
>>> More details are included in individual patches.  I broke up the helper into
>>> a series of patches to improve reviewabilty.
>>> 
>>
>> Would raw backend attached to a bridge mostly do the same?
>>   
>
> Well it doesn't really help with the issue of privileges which is what  
> this series is really about.
>
> Regards,
>
> Anthony Liguori

I note that by default you grant all users all access.
If you do that, just give them net cap admin already?

-- 
MST




[Qemu-devel] [PATCH 8/8] monitor: do_info_balloon(): use QError

2009-11-04 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 monitor.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 0e6e21b..423c6b5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1717,10 +1717,11 @@ static void do_info_balloon(Monitor *mon, QObject 
**ret_data)
 
 actual = qemu_balloon_status();
 if (kvm_enabled() && !kvm_has_sync_mmu())
-monitor_printf(mon, "Using KVM without synchronous MMU, "
-   "ballooning disabled\n");
+qemu_error_new(QERR_SER_UNAV,
+   "Using KVM without synchronous MMU, ballooning 
disabled",
+   NULL);
 else if (actual == 0)
-monitor_printf(mon, "Ballooning not activated in VM\n");
+qemu_error_new(QERR_SER_UNAV, "Ballooning not activated in VM", NULL);
 else
 *ret_data = QOBJECT(qint_from_int((int)(actual >> 20)));
 }
-- 
1.6.5.2.143.g8cc62





[Qemu-devel] [PATCH 7/8] qdev: Use QError for not found error

2009-11-04 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 hw/qdev.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index c7884d0..db86cb2 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,7 @@
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "qerror.h"
 
 static int qdev_hotplug = 0;
 
@@ -176,8 +177,8 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 /* find driver */
 info = qdev_find_info(NULL, driver);
 if (!info) {
-qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
-   driver);
+qemu_error_new(QERR_DEV_NFOUND, "device \"%(name)s\" not found",
+   "{ 'name': %s }", driver);
 return NULL;
 }
 if (info->no_user) {
-- 
1.6.5.2.143.g8cc62





[Qemu-devel] [PATCH 6/8] monitor: QError support

2009-11-04 Thread Luiz Capitulino
This commit adds QError support in the Monitor.

A QError member is added to the Monitor struct. This new member
stores error information and is also used to check if an error
has occurred when the called handlers returns.

Additionally, a new macro called qemu_error_new() is introduced.
It should be used in pace of qemu_error() to report errors.

When all conversion to qemu_error_new() is done, qemu_error() can
be turned private.

Basically, Monitor's error flow is something like this:

1. An error occurs in the handler, it calls qemu_error_new()
2. qemu_error_new() builds a new QError object and stores it in
   the Monitor struct
3. The handler returns
4. Top level Monitor code checks the Monitor struct and calls
   qerror_print() to print the error

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   43 ++-
 sysemu.h  |7 +++
 2 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 109ff5c..0e6e21b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -49,6 +49,7 @@
 #include "qlist.h"
 #include "qdict.h"
 #include "qstring.h"
+#include "qerror.h"
 
 //#define DEBUG
 //#define DEBUG_COMPLETION
@@ -103,6 +104,7 @@ struct Monitor {
 CPUState *mon_cpu;
 BlockDriverCompletionFunc *password_completion_cb;
 void *password_opaque;
+QError *error;
 QLIST_HEAD(,mon_fd_t) fds;
 QLIST_ENTRY(Monitor) entry;
 };
@@ -3146,6 +3148,18 @@ fail:
 return NULL;
 }
 
+static inline int monitor_has_error(const Monitor *mon)
+{
+return mon->error != NULL;
+}
+
+static void monitor_print_error(Monitor *mon)
+{
+qerror_print(mon->error);
+QDECREF(mon->error);
+mon->error = NULL;
+}
+
 static void monitor_handle_command(Monitor *mon, const char *cmdline)
 {
 QDict *qdict;
@@ -3171,7 +3185,10 @@ static void monitor_handle_command(Monitor *mon, const 
char *cmdline)
 cmd->mhandler.cmd(mon, qdict);
 }
 
-   qemu_errors_to_previous();
+if (monitor_has_error(mon))
+monitor_print_error(mon);
+
+qemu_errors_to_previous();
 
 out:
 QDECREF(qdict);
@@ -3622,3 +3639,27 @@ void qemu_error(const char *fmt, ...)
 break;
 }
 }
+
+void qemu_error_full(const char *name, const char *file, int linenr,
+ const char *desc, const char *fmt, ...)
+{
+va_list va;
+QError *qerror;
+
+assert(qemu_error_sink != NULL);
+
+va_start(va, fmt);
+qerror = qerror_from_info(name, file, linenr, desc, fmt, &va);
+va_end(va);
+
+switch (qemu_error_sink->dest) {
+case ERR_SINK_FILE:
+qerror_print(qerror);
+QDECREF(qerror);
+break;
+case ERR_SINK_MONITOR:
+assert(qemu_error_sink->mon->error == NULL);
+qemu_error_sink->mon->error = qerror;
+break;
+}
+}
diff --git a/sysemu.h b/sysemu.h
index 96804b4..20ab709 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -7,6 +7,7 @@
 #include "qemu-queue.h"
 #include "qemu-timer.h"
 #include "qdict.h"
+#include "qerror.h"
 
 #ifdef _WIN32
 #include 
@@ -71,6 +72,12 @@ void qemu_errors_to_file(FILE *fp);
 void qemu_errors_to_mon(Monitor *mon);
 void qemu_errors_to_previous(void);
 void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
+void qemu_error_full(const char *name, const char *file, int linenr,
+ const char *desc, const char *fmt, ...)
+ __attribute__ ((format(printf, 5, 6)));
+
+#define qemu_error_new(name, desc, fmt, ...) \
+qemu_error_full(name, __FILE__, __LINE__, desc, fmt, ## __VA_ARGS__)
 
 #ifdef _WIN32
 /* Polling handling */
-- 
1.6.5.2.143.g8cc62





[Qemu-devel] [PATCH 5/8] Introduce QError

2009-11-04 Thread Luiz Capitulino
QError is a high-level data type which represents an exception,
it stores the following error information:

- name   A generic error name (eg. "ServiceUnavailable")
- descriptionA detailed error description, which may contain
 references to run-time error data
- filename   The file name of where the error occurred
- line numberThe exact line number of the error
- run-time data  Run-time error data

The qerror_print() function should be used to properly format and
print the stored information to the right place, that is, to stderr
if the Monitor is not running, or to the Monitor's device otherwise.

The following functions are exported:

- qerror_new(): Create a new QError
- qerror_from_info(): Create a new QError from the specified error
  information
- qerror_print(): Print the specified QError

Signed-off-by: Luiz Capitulino 
---
 Makefile  |2 +-
 qerror.c  |  235 +
 qerror.h  |   39 ++
 qobject.h |1 +
 4 files changed, 276 insertions(+), 1 deletions(-)
 create mode 100644 qerror.c
 create mode 100644 qerror.h

diff --git a/Makefile b/Makefile
index 6fcbcaa..2dfaebd 100644
--- a/Makefile
+++ b/Makefile
@@ -135,7 +135,7 @@ obj-y += buffered_file.o migration.o migration-tcp.o 
qemu-sockets.o
 obj-y += qemu-char.o aio.o savevm.o
 obj-y += msmouse.o ps2.o
 obj-y += qdev.o qdev-properties.o
-obj-y += qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qjson.o
+obj-y += qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qjson.o qerror.o
 obj-y += qemu-config.o
 
 obj-$(CONFIG_BRLAPI) += baum.o
diff --git a/qerror.c b/qerror.c
new file mode 100644
index 000..7d3137f
--- /dev/null
+++ b/qerror.c
@@ -0,0 +1,235 @@
+/*
+ * QError: QEMU Error data-type.
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+#include "qint.h"
+#include "qjson.h"
+#include "qerror.h"
+#include "qstring.h"
+#include "sysemu.h"
+#include "qemu-common.h"
+
+static void qerror_destroy_obj(QObject *obj);
+
+static const QType qerror_type = {
+.code = QTYPE_QERROR,
+.destroy = qerror_destroy_obj,
+};
+
+/**
+ * qerror_new(): Create a new QError
+ *
+ * Return strong reference.
+ */
+QError *qerror_new(void)
+{
+QError *qerr;
+
+qerr = qemu_mallocz(sizeof(*qerr));
+QOBJECT_INIT(qerr, &qerror_type);
+
+return qerr;
+}
+
+/**
+ * qerror_from_info(): Create a new QError from error information
+ *
+ * The information consists of:
+ *
+ * - namegeneric error name
+ * - filethe file name of where the error occurred
+ * - linenr  the line number of where the error occurred
+ * - descdetailed description (see below)
+ * - fmt JSON printf-like format for 'specific data'
+ * - va  va_list of all arguments for 'specific data'
+ *
+ * Note that this is a low-level function, it is supposed to be called
+ * by higher-level functions or macros.
+ *
+ * The 'desc' parameter is a printf-like string, the format of the format
+ * string is:
+ *
+ * %(KEY)TYPE
+ *
+ * Where KEY is a QDict key and TYPE is the type of its value, KEY and
+ * its value must be passed to qerror_from_info().
+ *
+ * Valid types are:
+ *
+ * s  (string)
+ * d  (integer)
+ *
+ * Example:
+ *
+ * "foo error on device: %(device)s slot: %(slot_nr)d"
+ *
+ * A single percent sign can be printed if followed by a second one,
+ * for example:
+ *
+ * "running out of foo: %(foo)d%%"
+ *
+ * Return strong reference.
+ */
+QError *qerror_from_info(const char *name, const char *file, int linenr,
+ const char *desc, const char *fmt, va_list *va)
+{
+QError *qerr;
+
+qerr = qerror_new();
+qerr->name = name;
+qerr->file = file;
+qerr->linenr = linenr;
+qerr->desc = desc;
+if (fmt) {
+qerr->data = qobject_from_json_va(fmt, va);
+assert(qerr->data != NULL);
+}
+
+return qerr;
+}
+
+static char *get_substr(const char *start, const char *end)
+{
+char *str;
+size_t length;
+
+length = end - start + 1;
+str = qemu_malloc(length + 1);
+memcpy(str, start, length);
+str[length] = '\0';
+
+return str;
+}
+
+static void qerror_abort(const QError *qerr)
+{
+fprintf(stderr, " in '%s' at %s:%d\n", qerr->desc, 
qerr->file,qerr->linenr);
+abort();
+}
+
+#define ERROR_PREFIX "\n\nqerror: "
+
+static void type_error(const QError *qerr, int c)
+{
+fprintf(stderr, ERROR_PREFIX "invalid type '%c'", c);
+qerror_abort(qerr);
+}
+
+static void key_error(const QError *qerr, const char *key)
+{
+fprintf(stderr, ERROR_PREFIX "key '%s' not found in QDict ", key);
+fprintf(stderr, "call at %s:%d\n", qerr->file, qerr->linenr);
+abort();
+}
+
+static void parse_error(const QError *qerror, int c)
+{
+fprintf(stderr, ERROR_PREFIX "expected '%c'", c);
+qerror_abort(

[Qemu-devel] [PATCH 4/8] QString: Introduce qstring_append_int()

2009-11-04 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 qstring.c |8 
 qstring.h |2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/qstring.c b/qstring.c
index e422bd9..ad17769 100644
--- a/qstring.c
+++ b/qstring.c
@@ -75,6 +75,14 @@ void qstring_append(QString *qstring, const char *str)
 qstring->string[qstring->length] = 0;
 }
 
+void qstring_append_int(QString *qstring, int64_t value)
+{
+char num[32];
+
+snprintf(num, sizeof(num), "%" PRId64, value);
+qstring_append(qstring, num);
+}
+
 /**
  * qstring_append_chr(): Append a C char to a QString
  */
diff --git a/qstring.h b/qstring.h
index 43581de..c065331 100644
--- a/qstring.h
+++ b/qstring.h
@@ -1,6 +1,7 @@
 #ifndef QSTRING_H
 #define QSTRING_H
 
+#include 
 #include "qobject.h"
 
 typedef struct QString {
@@ -13,6 +14,7 @@ typedef struct QString {
 QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
 const char *qstring_get_str(const QString *qstring);
+void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
 QString *qobject_to_qstring(const QObject *obj);
-- 
1.6.5.2.143.g8cc62





[Qemu-devel] [PATCH 3/8] Add qstring_append_chr() unit-test

2009-11-04 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 check-qstring.c |   17 +
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/check-qstring.c b/check-qstring.c
index ea4dfd0..412038a 100644
--- a/check-qstring.c
+++ b/check-qstring.c
@@ -55,6 +55,22 @@ START_TEST(qstring_get_str_test)
 }
 END_TEST
 
+START_TEST(qstring_append_chr_test)
+{
+int i;
+QString *qstring;
+const char *str = "qstring append char unit-test";
+
+qstring = qstring_new();
+
+for (i = 0; str[i]; i++)
+qstring_append_chr(qstring, str[i]);
+
+fail_unless(strcmp(str, qstring_get_str(qstring)) == 0);
+QDECREF(qstring);
+}
+END_TEST
+
 START_TEST(qobject_to_qstring_test)
 {
 QString *qstring;
@@ -78,6 +94,7 @@ static Suite *qstring_suite(void)
 tcase_add_test(qstring_public_tcase, qstring_from_str_test);
 tcase_add_test(qstring_public_tcase, qstring_destroy_test);
 tcase_add_test(qstring_public_tcase, qstring_get_str_test);
+tcase_add_test(qstring_public_tcase, qstring_append_chr_test);
 tcase_add_test(qstring_public_tcase, qobject_to_qstring_test);
 
 return s;
-- 
1.6.5.2.143.g8cc62





[Qemu-devel] [PATCH 2/8] QString: Introduce qstring_append_chr()

2009-11-04 Thread Luiz Capitulino
It appends a C char to a QString.

Signed-off-by: Luiz Capitulino 
---
 qstring.c |   24 +++-
 qstring.h |1 +
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/qstring.c b/qstring.c
index 441a9e6..e422bd9 100644
--- a/qstring.c
+++ b/qstring.c
@@ -53,25 +53,39 @@ QString *qstring_from_str(const char *str)
 return qstring;
 }
 
-/* qstring_append(): Append a C string to a QString
- */
-void qstring_append(QString *qstring, const char *str)
+static void capacity_increase(QString *qstring, size_t len)
 {
-size_t len = strlen(str);
-
 if (qstring->capacity < (qstring->length + len)) {
 qstring->capacity += len;
 qstring->capacity *= 2; /* use exponential growth */
 
 qstring->string = qemu_realloc(qstring->string, qstring->capacity + 1);
 }
+}
+
+/* qstring_append(): Append a C string to a QString
+ */
+void qstring_append(QString *qstring, const char *str)
+{
+size_t len = strlen(str);
 
+capacity_increase(qstring, len);
 memcpy(qstring->string + qstring->length, str, len);
 qstring->length += len;
 qstring->string[qstring->length] = 0;
 }
 
 /**
+ * qstring_append_chr(): Append a C char to a QString
+ */
+void qstring_append_chr(QString *qstring, int c)
+{
+capacity_increase(qstring, 1);
+qstring->string[qstring->length++] = c;
+qstring->string[qstring->length] = 0;
+}
+
+/**
  * qobject_to_qstring(): Convert a QObject to a QString
  */
 QString *qobject_to_qstring(const QObject *obj)
diff --git a/qstring.h b/qstring.h
index 65905d4..43581de 100644
--- a/qstring.h
+++ b/qstring.h
@@ -14,6 +14,7 @@ QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
 const char *qstring_get_str(const QString *qstring);
 void qstring_append(QString *qstring, const char *str);
+void qstring_append_chr(QString *qstring, int c);
 QString *qobject_to_qstring(const QObject *obj);
 
 #endif /* QSTRING_H */
-- 
1.6.5.2.143.g8cc62





[Qemu-devel] [PATCH 1/8] QJSon: Introduce qobject_from_json_va()

2009-11-04 Thread Luiz Capitulino
Simple wrapper to parse_json() that accepts a va_list, will be
used by QError.

Signed-off-by: Luiz Capitulino 
---
 qjson.c |5 +
 qjson.h |2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qjson.c b/qjson.c
index 5f92996..02fcd83 100644
--- a/qjson.c
+++ b/qjson.c
@@ -723,3 +723,8 @@ QObject *qobject_from_jsonf(const char *string, size_t 
*length, ...)
 
 return obj;
 }
+
+QObject *qobject_from_json_va(const char *string, va_list *ap)
+{
+return parse_json(string, NULL, ap);
+}
diff --git a/qjson.h b/qjson.h
index 0c94954..da0d653 100644
--- a/qjson.h
+++ b/qjson.h
@@ -14,10 +14,12 @@
 #ifndef QJSON_H
 #define QJSON_H
 
+#include 
 #include "qobject.h"
 
 QObject *qobject_from_json(const char *string, size_t *length);
 QObject *qobject_from_jsonf(const char *string, size_t *length, ...)
 __attribute__((__format__ (__printf__, 1, 3)));
+QObject *qobject_from_json_va(const char *string, va_list *ap);
 
 #endif /* QJSON_H */
-- 
1.6.5.2.143.g8cc62





[Qemu-devel] [RFC 0/8]: QError v2

2009-11-04 Thread Luiz Capitulino
 Hi,

 I can't remember seeing updated versions of a RFC series, but this should
prevent Anthony's scripts from merging these patches.

 This new QError version has two major changes: the static error table has
been dropped and I'm using symbolic names instead of error codes.

 Now, a call to:

monitor_printf(mon, "husb: host usb device %d.%d is already open\n",
   bus_num, addr);

 Would become something like:

qemu_error_new('DeviceAlreadyOpen', "{ 'bus_num': %d, 'addr': %d }",
   bus_num, addr);

 Which is basically what Anthony and other people were asking for, the
only difference is that I'm not passing the symbolic name through the
dictionary. The reason is that I have the impression it's less general
(as it becomes mandatory to have a dict) and slightly more complicaded.

 The symbolic name can be freely defined, but we can have the common ones
in qerror.h.

 Hopefully this version addresses the most important issues.

 Luiz.




Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Anthony Liguori

Michael S. Tsirkin wrote:

On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote:
  

This series solves a problem that I've been struggling with for a few years now.
One of the best things about qemu is that it's possible to run guests as an
unprivileged user to improve security.  However, if you want to have your guests
communicate with the outside world, you're pretty much forced to run qemu as
root.

At least with KVM support, this is probably the most common use case which means
that most of our users are running qemu as root.  That's terrible.

We address this problem by introducing a new network backend: -net bridge.  This
backend is less flexible than -net tap because it relies on a helper with
elevated privileges to do the heavy lifting of allocating and attaching a tap
device to a bridge.  We use a special purpose helper because we don't want
to elevate the privileges of more generic tools like brctl.

>From a user perspective, to use bridged networking with a guest, you simply 
use:

  qemu -hda linux.img -net bridge -net nic

And assuming a bridge is defined named qemubr0 and the administrator has setup
permissions accordingly, it will Just Work.  My hope is that distributions will
do this work as part of the qemu packaging process such that for most users,
the out-of-the-box experience will also Just Work.

More details are included in individual patches.  I broke up the helper into
a series of patches to improve reviewabilty.



Would raw backend attached to a bridge mostly do the same?
  


Well it doesn't really help with the issue of privileges which is what 
this series is really about.


Regards,

Anthony Liguori





[Qemu-devel] Re: [PATCH] sparc32 fix carry flag handling (Solaris bootblk fix)

2009-11-04 Thread Blue Swirl
On Wed, Nov 4, 2009 at 1:58 AM, Artyom Tarasenko
 wrote:
> The page 108 of the SPARC Version 8 Architecture Manual describes
> that addcc and addxcc shall compute carry flag the same way.
> The page 110 claims the same about subcc and subxcc instructions.
> This patch fixes carry computation in corner cases and removes redundant code.
> The most visible effect of the patch is enabling Solaris boot when using OBP.

Thanks, applied.

Could you describe the steps how to boot Solaris with OBP? I'm sure
there are a lot of people who'd like to test if their favorite Sparc
Solaris programs work on QEMU.




Re: [Qemu-devel] Inquiry:Solaris 8 installation on QEMU

2009-11-04 Thread Blue Swirl
On Wed, Nov 4, 2009 at 1:50 AM, Artyom Tarasenko
 wrote:
> 2009/9/19 Blue Swirl :
>>  Even Sparc32 can't boot Solaris for some mysterious reason.
>
> Not so mysterious anymore! Mitch Bradley found that subcc instruction
> was not correctly setting carry flag in the case where both arguments
> were 0 and carry flag was previously set. Fixing the bug allowed to
> start booting Solaris 2.5.1 and Solaris 2.6 up to /sbin/init.
> Afterwards I found more corner cases in add(x)cc and sub(x)cc carry
> handling. Now Solaris 2.5.1 (
> http://tyom.blogspot.com/2009/10/greetings-professor-falken.html ) and
> 2.6 ( 
> http://tyom.blogspot.com/2009/11/another-week-another-solaris-version.html
> ) can be boot in a single user mode.

Awesome!

> The kernels of Solaris 8 & 9 can be boot too, but then they flood
> about spurious irq 10. It seems that the earlier Solaris versions are
> also suffering from spurious interrupts, because the boot process
> takes very long: ~7 hours on e8...@2.66ghz .

A bug in the system timer implementation?




[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.

2009-11-04 Thread Aurelien Jarno
On Wed, Nov 04, 2009 at 05:37:13PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 04, 2009 at 04:17:46PM +0100, Aurelien Jarno wrote:
> > On Wed, Nov 04, 2009 at 03:14:26PM +0900, Isaku Yamahata wrote:
> > > On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote:
> > > > > --- a/hw/pci_host.c
> > > > > +++ b/hw/pci_host.c
> > > > > @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## 
> > > > > __VA_ARGS__); } while (0)
> > > > >  #define PCI_DPRINTF(fmt, ...)
> > > > >  #endif
> > > > >  
> > > > > +static void pci_host_config_writel(void *opaque, target_phys_addr_t 
> > > > > addr,
> > > > > +   uint32_t val)
> > > > > +{
> > > > > +PCIHostState *s = opaque;
> > > > > +
> > > > > +#ifdef TARGET_WORDS_BIGENDIAN
> > > > > +val = bswap32(val);
> > > > > +#endif
> > > > 
> > > > I know you just copied it, but isn't this just
> > > > val = le32_to_cpu(val);
> > > > 
> > > > ?
> > > 
> > > Makes sense.
> >  
> > The original code is actually wrong, but le32_to_cpu(val), will break on
> > big endian hosts.
> > 
> > The fact is that QEMU doesn't emulate byteswap on buses. Hopefully on all
> > big endian machines we emulate, the PCI bus is always connected backward,
> > so we can simply do the byteswap depending on TARGET_WORDS_BIGENDIAN.
> > 
> > -- 
> > Aurelien Jarno  GPG: 1024D/F1BCDB73
> > aurel...@aurel32.net http://www.aurel32.net
> 
> Are you speaking about bit endian hosts with little endian guests?
> Ugh ... my head hurts. bswap32 is evil because there's no way to
> figure out what is converted to what. big to little? guest to host?
> 

This is not related to the host and guest endianess, but just of the
way the PCI bus is connected to the CPU on all the big endian guest we 
emulate.

On theses machine, a byteswap is needed when transferring a word between
the PCI bus and the CPU.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Michael S. Tsirkin
On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote:
> This series solves a problem that I've been struggling with for a few years 
> now.
> One of the best things about qemu is that it's possible to run guests as an
> unprivileged user to improve security.  However, if you want to have your 
> guests
> communicate with the outside world, you're pretty much forced to run qemu as
> root.
> 
> At least with KVM support, this is probably the most common use case which 
> means
> that most of our users are running qemu as root.  That's terrible.
> 
> We address this problem by introducing a new network backend: -net bridge.  
> This
> backend is less flexible than -net tap because it relies on a helper with
> elevated privileges to do the heavy lifting of allocating and attaching a tap
> device to a bridge.  We use a special purpose helper because we don't want
> to elevate the privileges of more generic tools like brctl.
> 
> >From a user perspective, to use bridged networking with a guest, you simply 
> >use:
> 
>   qemu -hda linux.img -net bridge -net nic
> 
> And assuming a bridge is defined named qemubr0 and the administrator has setup
> permissions accordingly, it will Just Work.  My hope is that distributions 
> will
> do this work as part of the qemu packaging process such that for most users,
> the out-of-the-box experience will also Just Work.
> 
> More details are included in individual patches.  I broke up the helper into
> a series of patches to improve reviewabilty.

Would raw backend attached to a bridge mostly do the same?

-- 
MST




Re: [Qemu-devel] [PATCH] don't call reset functions on cpu initialization\

2009-11-04 Thread Marcelo Tosatti
On Tue, Nov 03, 2009 at 05:50:05PM -0200, Glauber Costa wrote:
> There is absolutely no need to call reset functions when initializing
> devices. Since we are already registering them, calling qemu_system_reset()
> should suffice. Actually, it is what happens when we reboot the machine,
> and using the same process instead of a special case semantics will even
> allow us to find bugs easier.
> 
> Furthermore, the fact that we initialize things like the cpu quite early,
> leads to the need to introduce synchronization stuff like qemu_system_cond.
> This patch removes it entirely. All we need to do is call qemu_system_reset()
> only when we're already sure the system is up and running
> 
> I tested it with qemu (with and without io-thread) and qemu-kvm, and it
> seems to be doing okay - although qemu-kvm uses a slightly different patch.
> 
> Signed-off-by: Glauber Costa 
> ---
>  hw/apic.c|1 -
>  hw/e1000.c   |1 -
>  hw/hpet.c|1 -
>  hw/i8254.c   |2 --
>  hw/ide/piix.c|1 -
>  hw/piix4.c   |1 -
>  hw/piix_pci.c|1 -
>  hw/rtl8139.c |2 +-
>  hw/serial.c  |1 -
>  hw/usb-ohci.c|1 -
>  hw/usb-uhci.c|1 -
>  hw/vga.c |1 -
>  target-i386/helper.c |1 -
>  vl.c |   15 +--
>  14 files changed, 2 insertions(+), 28 deletions(-)
> 
> diff --git a/hw/apic.c b/hw/apic.c
> index c89008e..87e7dc0 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -981,7 +981,6 @@ int apic_init(CPUState *env)
>  s->id = env->cpuid_apic_id;
>  s->cpu_env = env;
>  
> -apic_reset(s);
>  msix_supported = 1;
>  
>  /* XXX: mapping more APICs at the same memory location */
> diff --git a/hw/e1000.c b/hw/e1000.c
> index 028afd1..8fb299d 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -1113,7 +1113,6 @@ static int pci_e1000_init(PCIDevice *pci_dev)
>  qemu_format_nic_info_str(d->vc, macaddr);
>  
>  vmstate_register(-1, &vmstate_e1000, d);
> -e1000_reset(d);
>  
>  if (!pci_dev->qdev.hotplugged) {
>  static int loaded = 0;
> diff --git a/hw/hpet.c b/hw/hpet.c
> index 64163bd..6f39711 100644
> --- a/hw/hpet.c
> +++ b/hw/hpet.c
> @@ -577,7 +577,6 @@ void hpet_init(qemu_irq *irq) {
>  HPETTimer *timer = &s->timer[i];
>  timer->qemu_timer = qemu_new_timer(vm_clock, hpet_timer, timer);
>  }
> -hpet_reset(s);
>  vmstate_register(-1, &vmstate_hpet, s);
>  qemu_register_reset(hpet_reset, s);
>  /* HPET Area */
> diff --git a/hw/i8254.c b/hw/i8254.c
> index 5c49e6e..faaa884 100644
> --- a/hw/i8254.c
> +++ b/hw/i8254.c
> @@ -513,7 +513,5 @@ PITState *pit_init(int base, qemu_irq irq)
>  register_ioport_write(base, 4, 1, pit_ioport_write, pit);
>  register_ioport_read(base, 3, 1, pit_ioport_read, pit);
>  
> -pit_reset(pit);
> -
>  return pit;
>  }
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index a17bf59..60b37a3 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -120,7 +120,6 @@ static int pci_piix_ide_initfn(PCIIDEState *d)
>  pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>  
>  qemu_register_reset(piix3_reset, d);
> -piix3_reset(d);
>  
>  pci_register_bar(&d->dev, 4, 0x10, PCI_ADDRESS_SPACE_IO, bmdma_map);
>  
> diff --git a/hw/piix4.c b/hw/piix4.c
> index a6aea15..f75951b 100644
> --- a/hw/piix4.c
> +++ b/hw/piix4.c
> @@ -97,7 +97,6 @@ static int piix4_initfn(PCIDevice *d)
>  PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // 
> header_type = PCI_multifunction, generic
>  
>  piix4_dev = d;
> -piix4_reset(d);
>  qemu_register_reset(piix4_reset, d);
>  return 0;
>  }
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index ed036fe..20d834f 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -341,7 +341,6 @@ static int piix3_initfn(PCIDevice *dev)
>  pci_conf[PCI_HEADER_TYPE] =
>  PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // 
> header_type = PCI_multifunction, generic
>  
> -piix3_reset(d);
>  qemu_register_reset(piix3_reset, d);
>  return 0;
>  }
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index d26df48..27cc618 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -3331,7 +3331,7 @@ static int pci_rtl8139_init(PCIDevice *dev)
> PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_map);
>  
>  qemu_macaddr_default_if_unset(&s->conf.macaddr);
> -rtl8139_reset(&s->dev.qdev);
> +
>  s->vc = qemu_new_vlan_client(NET_CLIENT_TYPE_NIC,
>   s->conf.vlan, s->conf.peer,
>   dev->qdev.info->name, dev->qdev.id,
> diff --git a/hw/serial.c b/hw/serial.c
> index 9353201..fa12dcc 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -725,7 +725,6 @@ static void serial_init_core(SerialState *s)
>  s->transmit_timer = qemu_new_timer(vm_clock, (QEMUTimerCB *) 
> serial_xmit, s);
>  
>  qemu_r

Re: [Qemu-devel] Re: [PATCH] Add VirtIO Frame Buffer Support

2009-11-04 Thread Anthony Liguori

Vincent Hanquez wrote:

On Tue, Nov 03, 2009 at 11:38:18AM +0200, Avi Kivity wrote:
  

On 11/03/2009 01:25 PM, Vincent Hanquez wrote:


not sure if i'm missing the point here, but couldn't it be hypothetically
extended to stuff 3d (or video&  more 2d accel ?) commands too ? I can't
imagine the cirrus or stdvga driver be able to do that ever ;)
   
  

cirrus has pretty good 2d acceleration.  3D is a mega-project though.



absolutely huge indeed, but still alexander's code is pretty much the
only way, to start such a project. with maybe added benefits on more
and easier 2d acceleration.

or otherwise wait for SR-IOV graphics cards (or similar tech)...
  


I think the real question is do we paravirtualize a VGA device or a 
framebuffer.


Obviously, the advantage of doing a framebuffer is that it works for s390.

A VGA device has better backwards compatibility on PCs although it's 
obviously more complex.  In an ideal world, we could expose the virtio 
framebuffer device as part of PCI device that was also VGA capable 
(virtio-pci-vga transport?).


But then there's QXL on the horizon which complicates matters further.

I'd say that virtio-fb should just focus on the s390 use case for now.  
Let things evolve as needed.


Regards,

Anthony Liguori






[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

2009-11-04 Thread Amit Shah
On (Wed) Nov 04 2009 [15:30:21], Jan Kiszka wrote:
> 
> I've nothing against your patch, specifically as it removes an obviously
> no longer needed workaround, not a feature. I just want to make sure
> that the current behavior is not only there by chance.

Sure; I understand that. I'll be looking more at this code to free it
from any surprises.

Amit




Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Anthony Liguori

Alexander Graf wrote:

Well I'm not that familiar with the bridging stuff as I'm rather scared
by it myself, but last time I tried if I

# brctl addif br0 eth0
# ifconfig br0 up

eth0 stopped working, so I had to stop network manager, assign an IP to
br0 manually and hope network manager doesn't kick in again. I don't see
how we can solve that easily, as most people will want to use NM.
  


netcf will address this properly because it will allow NM to understand 
bridges and provide a common framework for NM to know about when changes 
are made to static networking configuration.


Regards,

Anthony Liguori





[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.

2009-11-04 Thread Michael S. Tsirkin
On Wed, Nov 04, 2009 at 04:17:46PM +0100, Aurelien Jarno wrote:
> On Wed, Nov 04, 2009 at 03:14:26PM +0900, Isaku Yamahata wrote:
> > On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote:
> > > > --- a/hw/pci_host.c
> > > > +++ b/hw/pci_host.c
> > > > @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## 
> > > > __VA_ARGS__); } while (0)
> > > >  #define PCI_DPRINTF(fmt, ...)
> > > >  #endif
> > > >  
> > > > +static void pci_host_config_writel(void *opaque, target_phys_addr_t 
> > > > addr,
> > > > +   uint32_t val)
> > > > +{
> > > > +PCIHostState *s = opaque;
> > > > +
> > > > +#ifdef TARGET_WORDS_BIGENDIAN
> > > > +val = bswap32(val);
> > > > +#endif
> > > 
> > > I know you just copied it, but isn't this just
> > >   val = le32_to_cpu(val);
> > > 
> > > ?
> > 
> > Makes sense.
>  
> The original code is actually wrong, but le32_to_cpu(val), will break on
> big endian hosts.
> 
> The fact is that QEMU doesn't emulate byteswap on buses. Hopefully on all
> big endian machines we emulate, the PCI bus is always connected backward,
> so we can simply do the byteswap depending on TARGET_WORDS_BIGENDIAN.
> 
> -- 
> Aurelien JarnoGPG: 1024D/F1BCDB73
> aurel...@aurel32.net http://www.aurel32.net

Are you speaking about bit endian hosts with little endian guests?
Ugh ... my head hurts. bswap32 is evil because there's no way to
figure out what is converted to what. big to little? guest to host?


-- 
MST




[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.

2009-11-04 Thread Aurelien Jarno
On Wed, Nov 04, 2009 at 03:14:26PM +0900, Isaku Yamahata wrote:
> On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote:
> > > --- a/hw/pci_host.c
> > > +++ b/hw/pci_host.c
> > > @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); 
> > > } while (0)
> > >  #define PCI_DPRINTF(fmt, ...)
> > >  #endif
> > >  
> > > +static void pci_host_config_writel(void *opaque, target_phys_addr_t addr,
> > > +   uint32_t val)
> > > +{
> > > +PCIHostState *s = opaque;
> > > +
> > > +#ifdef TARGET_WORDS_BIGENDIAN
> > > +val = bswap32(val);
> > > +#endif
> > 
> > I know you just copied it, but isn't this just
> > val = le32_to_cpu(val);
> > 
> > ?
> 
> Makes sense.
 
The original code is actually wrong, but le32_to_cpu(val), will break on
big endian hosts.

The fact is that QEMU doesn't emulate byteswap on buses. Hopefully on all
big endian machines we emulate, the PCI bus is always connected backward,
so we can simply do the byteswap depending on TARGET_WORDS_BIGENDIAN.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




[Qemu-devel] Re: [PATCH] ARM NEON shift emulation fix

2009-11-04 Thread Daniel Gutson

Hi,
  friendly reminder about this patch.

Thanks,
Daniel.

Daniel Gutson wrote:

Any update on this?

Thanks,
Daniel.


Daniel Gutson wrote:

Hi,
  the attached patch fixes a bug that caused some NEON shift 
operations to shift a wrong amount of bytes.


The problem was that a variable holding an immediate value 
representing the amount of bits to shift was later overwritten with 
another value (used for something different) within a loop.


Please commit this for me if approved, since I don't have write access.

Thanks!
Daniel.

---
2009-08-21  Daniel Gutson  

* target-arm/translate.c (disas_neon_data_insn): Fixed
shift operand.





--
Daniel Gutson
CodeSourcery
www.codesourcery.com




Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Alexander Graf
Anthony Liguori wrote:
> Alexander Graf wrote:
>> Yeah. Worse than the "run as root" part is the "it's hard" part
>> though. I hate how I feel when I try to explain someone how to use
>> non-slirp networking :-(.
>>
>> The response to that is then usually "oh whatever, it's too
>> complicated anyways".
>
> I agree and it's a problem I would like to solve too.
>
>>> And assuming a bridge is defined named qemubr0 and the administrator
>>> has setup
>>> permissions accordingly, it will Just Work.  My hope is that
>>> distributions will
>>> do this work as part of the qemu packaging process such that for
>>> most users,
>>> the out-of-the-box experience will also Just Work.
>>
>> Yeah, that won't work as easily.
>>
>> When your customer has 2 NICs this will already break. But let's
>> imagine we only have a single NIC.
>>
>> So that NIC is a wifi card. When I set up the qemubr0 bridge for that
>> one now, how does network manager configure my wifi access? It can't
>> use the bridge device, as that doesn't have wifi extensions. It also
>> can't use the wifi device, because setting up networking on that will
>> break.
>>
>> IMHO the only customer friendly choice I see is the ugly way. The way
>> VMware and Vbox do it.
>> To make it a bit less ugly, maybe we could create some way to "glue"
>> a tap device and an eth together, the same way the bridge code does,
>> just without the extra device.
>
> I don't think that helps either.  At the end of the day, this is
> really a policy decision.  You want to expose the subset of
> functionality that the majority of your users require.  Inevitably,
> you'll leave some users who need more complex setups on their own.
>
> For instance, users with one NIC are certainly common.  Two or more
> NICs are also pretty common.  But what about people that require
> guests to be on separate VLANs?
>
> This is where management tools come in.  When running qemu by hand,
> the management tool is the distro more or less as they will be
> choosing the default policies.  What we need to think about is how to
> make sure we can seamlessly integrate with whatever policies they want
> to implement.
>
> I think the one thing we could add is a configurable message to print
> when a user tries to use a bridge that isn't configured yet.  For
> instance, if a user runs qemu without a bridge setup, what would be
> nice is the following:
>
> $ qemu -net bridge -net nic -hda ~/images/linux.img
> qemu: error running bridge helper
>
> You currently do not have 'qemubr0' configured.  To setup qemubr0, run
> (as root):
>  zypper install qemu-network-setup
>  qemu-network-setup qemubr0
>
> $ sudo qemu-network-setup
> Which interface do you want to configure [qemubr0]: qemubr0
> Do you want to configure qemubr0 as NAT or Bridge [NAT]: Bridge
> Which physical interface do you want to bridge qemubr0 to [eth0]: eth0
> qemubr0 is now configured.
>
> I expect that this is going to be different for every distro.  netcf
> my make this easier but for now, I think this is the most realistic
> approach.

Well I'm not that familiar with the bridging stuff as I'm rather scared
by it myself, but last time I tried if I

# brctl addif br0 eth0
# ifconfig br0 up

eth0 stopped working, so I had to stop network manager, assign an IP to
br0 manually and hope network manager doesn't kick in again. I don't see
how we can solve that easily, as most people will want to use NM.

Alex





Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Anthony Liguori

Alexander Graf wrote:
Yeah. Worse than the "run as root" part is the "it's hard" part 
though. I hate how I feel when I try to explain someone how to use 
non-slirp networking :-(.


The response to that is then usually "oh whatever, it's too 
complicated anyways".


I agree and it's a problem I would like to solve too.

And assuming a bridge is defined named qemubr0 and the administrator 
has setup
permissions accordingly, it will Just Work.  My hope is that 
distributions will
do this work as part of the qemu packaging process such that for most 
users,

the out-of-the-box experience will also Just Work.


Yeah, that won't work as easily.

When your customer has 2 NICs this will already break. But let's 
imagine we only have a single NIC.


So that NIC is a wifi card. When I set up the qemubr0 bridge for that 
one now, how does network manager configure my wifi access? It can't 
use the bridge device, as that doesn't have wifi extensions. It also 
can't use the wifi device, because setting up networking on that will 
break.


IMHO the only customer friendly choice I see is the ugly way. The way 
VMware and Vbox do it.
To make it a bit less ugly, maybe we could create some way to "glue" a 
tap device and an eth together, the same way the bridge code does, 
just without the extra device.


I don't think that helps either.  At the end of the day, this is really 
a policy decision.  You want to expose the subset of functionality that 
the majority of your users require.  Inevitably, you'll leave some users 
who need more complex setups on their own.


For instance, users with one NIC are certainly common.  Two or more NICs 
are also pretty common.  But what about people that require guests to be 
on separate VLANs?


This is where management tools come in.  When running qemu by hand, the 
management tool is the distro more or less as they will be choosing the 
default policies.  What we need to think about is how to make sure we 
can seamlessly integrate with whatever policies they want to implement.


I think the one thing we could add is a configurable message to print 
when a user tries to use a bridge that isn't configured yet.  For 
instance, if a user runs qemu without a bridge setup, what would be nice 
is the following:


$ qemu -net bridge -net nic -hda ~/images/linux.img
qemu: error running bridge helper

You currently do not have 'qemubr0' configured.  To setup qemubr0, run 
(as root):

 zypper install qemu-network-setup
 qemu-network-setup qemubr0

$ sudo qemu-network-setup
Which interface do you want to configure [qemubr0]: qemubr0
Do you want to configure qemubr0 as NAT or Bridge [NAT]: Bridge
Which physical interface do you want to bridge qemubr0 to [eth0]: eth0
qemubr0 is now configured.

I expect that this is going to be different for every distro.  netcf my 
make this easier but for now, I think this is the most realistic approach.


Regards,

Anthony Liguori





RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper

2009-11-04 Thread Krumme, Chris
Hello Anthony, 

> -Original Message-
> From: Anthony Liguori [mailto:anth...@codemonkey.ws] 
> Sent: Wednesday, November 04, 2009 8:23 AM
> To: Krumme, Chris
> Cc: qemu-devel@nongnu.org; Mark McLoughlin; Arnd Bergmann; 
> Michael Tsirkin; Juan Quintela; Dustin Kirkland
> Subject: Re: [Qemu-devel] [PATCH 2/4] Add access control 
> support toqemu-bridge-helper
> 
> Krumme, Chris wrote:
> > Hello Anthony,
> >
> > Cool patch series.
> >   
> 
> Thanks.
> 
> >> +cmd = ptr;
> >> +arg = strchr(cmd, ' ');
> >> +if (arg == NULL) {
> >> +arg = strchr(cmd, '\t');
> >> +}
> >> +
> >> +if (arg == NULL) {
> >> +fprintf(stderr, "Invalid config line:\n  %s\n", line);
> >> +fclose(f);
> >> +errno = EINVAL;
> >> +return -1;
> >> +}
> >> +
> >> +*arg = 0;
> >> 
> >
> > No check is made for arg being in bounds.
> >   
> 
> I don't get it.  arg is either going to be NULL (no ' ' or 
> '\t' found in 
> the string) or it will point to the first ' ' or '\t' in the 
> string.  It 

My concern is that the first space or tab may not be in the first
sizeof(line) characters.

Thanks

Chris

> will always be in bound in this second case and the first case is 
> handled by the if().
> 
> Regards,
> 
> Anthony Liguori
> 




[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

2009-11-04 Thread Jan Kiszka
Amit Shah wrote:
> On (Wed) Nov 04 2009 [10:39:39], Jan Kiszka wrote:
>> Amit Shah wrote:
>>> On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote:
 Amit Shah wrote:
> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
>> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
>>> Amit Shah wrote:
 The initial_reset sent to chardevs doesn't do much other than setting
 a bool to true. Char devices are interested in the open event and
 that gets sent whenever the device is opened.
>>> Have you checked with the monitor in all use cases (dedicated & muxed
>>> console, stdio & SDL console, etc.)? It was introduced once to fix a
>> I've checked with -monitor stdio, monitor in SDL and also chardevs using
>> unix sockets.
>>
>> I've not tried mux yet; I'll try that and report back. BTW if it ends up
>> not working with this patch, it'd be broken in the current master as
>> well.
> I tried with:
>
> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
>
> The monitor prompt shows up as does the serial output.
>
> (btw I've also tried closing and opening char devs and that works fine
> too)
 That sounds good. Then something must have changed since 2970a6c943, do
 you see what?
>>> I think that depended on the resets being sent. I've now removed the
>>> need for resets altogether.
>> No, this is in fact the reason why we no longer need it:
>>
>>  9a1e948129 (Introduce contexts for asynchronous callbacks)
>>
>> As the initial reset of the char device that is marked pending on open
>> is now no longer consumed by the IDE initialization, we can actually
>> drop the later regeneration via qemu_chr_initial_reset. I just hope this
>> stays like it is...
> 
> I tested this even on a tree that doesn't have this patch.

The whole things was (and maybe still is) fragile. So you have to take
care what further patches with side effects you include. If you revert
9a1e948129 and remove qemu_chr_initial_reset, you get the old bug back
again, ie. no prompt.

> 
> I haven't really delved deep to see why this was added earlier -- the
> commit log only says very little. Plus my testing with the current tree
> works fine so I'm happy to mention these things in the commit log.

I've nothing against your patch, specifically as it removes an obviously
no longer needed workaround, not a feature. I just want to make sure
that the current behavior is not only there by chance.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

2009-11-04 Thread Anthony Liguori

Krumme, Chris wrote:

Do you need to mention the default name qemubr0 here?
  


Good suggestion.

Regards,

Anthony Liguori




Re: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper

2009-11-04 Thread Anthony Liguori

Krumme, Chris wrote:

Hello Anthony,

Cool patch series.
  


Thanks.


+cmd = ptr;
+arg = strchr(cmd, ' ');
+if (arg == NULL) {
+arg = strchr(cmd, '\t');
+}
+
+if (arg == NULL) {
+fprintf(stderr, "Invalid config line:\n  %s\n", line);
+fclose(f);
+errno = EINVAL;
+return -1;
+}
+
+*arg = 0;



No check is made for arg being in bounds.
  


I don't get it.  arg is either going to be NULL (no ' ' or '\t' found in 
the string) or it will point to the first ' ' or '\t' in the string.  It 
will always be in bound in this second case and the first case is 
handled by the if().


Regards,

Anthony Liguori




Re: [Qemu-devel] Re: [PATCH] Add VirtIO Frame Buffer Support

2009-11-04 Thread Vincent Hanquez
On Tue, Nov 03, 2009 at 11:38:18AM +0200, Avi Kivity wrote:
> On 11/03/2009 01:25 PM, Vincent Hanquez wrote:
>> not sure if i'm missing the point here, but couldn't it be hypothetically
>> extended to stuff 3d (or video&  more 2d accel ?) commands too ? I can't
>> imagine the cirrus or stdvga driver be able to do that ever ;)
>>
>
> cirrus has pretty good 2d acceleration.  3D is a mega-project though.

absolutely huge indeed, but still alexander's code is pretty much the
only way, to start such a project. with maybe added benefits on more
and easier 2d acceleration.

or otherwise wait for SR-IOV graphics cards (or similar tech)...

-- 
Vincent




RE: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

2009-11-04 Thread Krumme, Chris
Hello Anthony,

Now that I have read the whole series I say again great patch. 

> -Original Message-
> From: 
> qemu-devel-bounces+chris.krumme=windriver@nongnu.org 
> [mailto:qemu-devel-bounces+chris.krumme=windriver@nongnu.o
> rg] On Behalf Of Anthony Liguori
> Sent: Tuesday, November 03, 2009 6:28 PM
> To: qemu-devel@nongnu.org
> Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin 
> Kirkland; Michael Tsirkin; Juan Quintela
> Subject: [Qemu-devel] [PATCH 4/4] Add support for -net bridge
> 
> The most common use of -net tap is to connect a tap device to 
> a bridge.  This
> requires the use of a script and running qemu as root in 
> order to allocate a
> tap device to pass to the script.
> 
> This model is great for portability and flexibility but it's 
> incredibly
> difficult to eliminate the need to run qemu as root.  The 
> only really viable
> mechanism is to use tunctl to create a tap device, attach it 
> to a bridge as
> root, and then hand that tap device to qemu.  The problem 
> with this mechanism
> is that it requires administrator intervention whenever a 
> user wants to create
> a guest.
> 
> By essentially writing a helper that implements the most 
> common qemu-ifup
> script that can be safely given cap_net_admin, we can 
> dramatically simplify
> things for non-privileged users.  We still support -net tap 
> as a mechanism
> for advanced users and backwards compatibility.
> 
> Currently, this is very Linux centric but there's really no 
> reason why it
> couldn't be extended for other Unixes.
> 
> A typical invocation of -net bridge would be:
> 
>   qemu -net bridge -net nic,model=virtio
> 
> The default bridge that we attach to is qemubr0.  The 
> thinking is that a distro
> could preconfigure such an interface to allow out-of-the-box 
> bridged networking.
> 
> Alternatively, if a user wants to use a different bridge, 
> they can say:
> 
>   qemu -net bridge,br=br0 -net nic,model=virtio
> 
> Signed-off-by: Anthony Liguori 
> ---
>  configure   |1 +
>  net.c   |   20 +++-
>  net/tap.c   |  142 
> +++
>  net/tap.h   |2 +
>  qemu-options.hx |3 +
>  5 files changed, 167 insertions(+), 1 deletions(-)
> 
> diff --git a/configure b/configure
> index 7c9d3a2..55a1a4f 100755
> --- a/configure
> +++ b/configure
> @@ -1896,6 +1896,7 @@ echo >> $config_host_mak
>  
>  echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> 
> $config_host_mak
>  echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak
> +echo "CONFIG_QEMU_HELPERDIR=\"$prefix/libexec\"" >> $config_host_mak
>  
>  case "$cpu" in
>
> i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p
> pc|ppc64|s390|sparc|sparc64)
> diff --git a/net.c b/net.c
> index 37662c6..54a7a5b 100644
> --- a/net.c
> +++ b/net.c
> @@ -2541,6 +2541,22 @@ static struct {
>  },
>  { /* end of list */ }
>  },
> +}, {
> +.type = "bridge",
> +.init = net_init_bridge,
> +.desc = {
> +NET_COMMON_PARAMS_DESC,
> +{
> +.name = "br",
> +.type = QEMU_OPT_STRING,
> +.help = "bridge name",
> +}, {
> +.name = "helper",
> +.type = QEMU_OPT_STRING,
> +.help = "command to execute to configure bridge",
> +},
> +{ /* end of list */ }
> +},
>  },
>  { /* end of list */ }
>  };
> @@ -2565,7 +2581,8 @@ int net_client_init(Monitor *mon, 
> QemuOpts *opts, int is_netdev)
>  #ifdef CONFIG_VDE
>  strcmp(type, "vde") != 0 &&
>  #endif
> -strcmp(type, "socket") != 0) {
> +strcmp(type, "socket") != 0 &&
> +strcmp(type, "bridge") != 0) {
>  qemu_error("The '%s' network backend type is not 
> valid with -netdev\n",
> type);
>  return -1;
> @@ -2641,6 +2658,7 @@ static int net_host_check_device(const 
> char *device)
>  #ifdef CONFIG_VDE
> ,"vde"
>  #endif
> +   , "bridge"
>  };
>  for (i = 0; i < sizeof(valid_param_list) / sizeof(char *); i++) {
>  if (!strncmp(valid_param_list[i], device,
> diff --git a/net/tap.c b/net/tap.c
> index bdb4a15..f5abed6 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -436,3 +436,145 @@ int net_init_tap(QemuOpts *opts, 
> Monitor *mon, const char *name, VLANState *vlan
>  
>  return 0;
>  }
> +
> +#define DEFAULT_BRIDGE_INTERFACE "qemubr0"
> +#define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR 
> "/qemu-bridge-helper"
> +
> +static int recv_fd(int c)
> +{
> +int fd;
> +uint8_t msgbuf[CMSG_SPACE(sizeof(fd))];
> +struct msghdr msg = {
> +.msg_control = msgbuf,
> +.msg_controllen = sizeof(msgbuf),
> +};
> +struct cmsghdr *cmsg;
> +struct iovec iov;
> +uint8_t req[1];
> +ssize_

RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper

2009-11-04 Thread Krumme, Chris
Hello Anthony,

Cool patch series.
 

> -Original Message-
> From: 
> qemu-devel-bounces+chris.krumme=windriver@nongnu.org 
> [mailto:qemu-devel-bounces+chris.krumme=windriver@nongnu.o
> rg] On Behalf Of Anthony Liguori
> Sent: Tuesday, November 03, 2009 6:28 PM
> To: qemu-devel@nongnu.org
> Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin 
> Kirkland; Michael Tsirkin; Juan Quintela
> Subject: [Qemu-devel] [PATCH 2/4] Add access control support 
> toqemu-bridge-helper
> 
> We go to great lengths to restrict ourselves to just 
> cap_net_admin as an OS
> enforced security mechanism.  However, we further restrict 
> what we allow users
> to do to simply adding a tap device to a bridge interface by 
> virtue of the fact
> that this is the only functionality we expose.
> 
> This is not good enough though.  An administrator is likely 
> to want to restrict
> the bridges that an unprivileged user can access, in 
> particular, to restrict
> an unprivileged user from putting a guest on what should be 
> isolated networks.
> 
> This patch implements a ACL mechanism that is enforced by 
> qemu-bridge-helper.
> The ACLs are fairly simple whitelist/blacklist mechanisms 
> with a wildcard of
> 'all'.
> 
> An interesting feature of this ACL mechanism is that you can 
> include external
> ACL files.  The main reason to support this is so that you 
> can set different
> file system permissions on those external ACL files.  This allows an
> administrator to implement rather sophisicated ACL policies 
> based on user/group
> policies via the file system.
> 
> If we fail to include an acl file, we are silent about it 
> making this mechanism
> work pretty seamlessly.  As an example:
> 
> /etc/qemu/bridge.conf root:qemu 0640
> 
>  deny all
>  allow br0
>  include /etc/qemu/alice.conf
>  include /etc/qemu/bob.conf
> 
> /etc/qemu/alice.conf root:alice 0640
>  allow br1
> 
> /etc/qemu/bob.conf root:bob 0640
>  allow br2
> 
> This ACL pattern allows any user in the qemu group to get a tap device
> connected to br0 (which is bridged to the physical network).
> 
> Users in the alice group can additionally get a tap device 
> connected to br1.
> This allows br1 to act as a private bridge for the alice group.
> 
> Users in the bob group can additionally get a tap device 
> connected to br2.
> This allows br2 to act as a private bridge for the bob group.
> 
> Under no circumstance can the bob group get access to br1 or 
> can the alice
> group get access to br2.
> 
> Signed-off-by: Anthony Liguori 
> ---
>  configure|1 +
>  qemu-bridge-helper.c |  138 
> ++
>  2 files changed, 139 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index a341e77..7c98257 100755
> --- a/configure
> +++ b/configure
> @@ -1864,6 +1864,7 @@ printf " '%s'" "$0" "$@" >> $config_host_mak
>  echo >> $config_host_mak
>  
>  echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> 
> $config_host_mak
> +echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak
>  
>  case "$cpu" in
>
> i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p
> pc|ppc64|s390|sparc|sparc64)
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index f10d37c..0d059ed 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -33,6 +33,106 @@
>  
>  #include "net/tap-linux.h"
>  
> +#define MAX_ACLS (128)
> +#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> +
> +enum {
> +ACL_ALLOW = 0,
> +ACL_ALLOW_ALL,
> +ACL_DENY,
> +ACL_DENY_ALL,
> +};
> +
> +typedef struct ACLRule
> +{
> +int type;
> +char iface[IFNAMSIZ];
> +} ACLRule;
> +
> +static int parse_acl_file(const char *filename, ACLRule 
> *acls, int *pacl_count)
> +{
> +int acl_count = *pacl_count;
> +FILE *f;
> +char line[4096];
> +
> +f = fopen(filename, "r");
> +if (f == NULL) {
> +return -1;
> +}
> +
> +while (acl_count != MAX_ACLS &&
> +   fgets(line, sizeof(line), f) != NULL) {
> +char *ptr = line;
> +char *cmd, *arg, *argend;
> +
> +while (isspace(*ptr)) {
> +ptr++;
> +}
> +
> +/* skip comments and empty lines */
> +if (*ptr == '#' || *ptr == 0) {
> +continue;
> +}
> +
> +cmd = ptr;
> +arg = strchr(cmd, ' ');
> +if (arg == NULL) {
> +arg = strchr(cmd, '\t');
> +}
> +
> +if (arg == NULL) {
> +fprintf(stderr, "Invalid config line:\n  %s\n", line);
> +fclose(f);
> +errno = EINVAL;
> +return -1;
> +}
> +
> +*arg = 0;

No check is made for arg being in bounds.

Thanks

Chris

> +arg++;
> +while (isspace(*arg)) {
> +arg++;
> +}
> +
> +argend = arg + strlen(arg);
> +while (arg != argend && isspace(*(argend - 1))) {
> +argend--;
> +}
> +*argend = 

[Qemu-devel][PATCH v2] Added read access to several e1000 hw registers

2009-11-04 Thread Kay Ackermann
The e1000 hardware allows read access to several registers which qemu doesn't 
allow yet.

Signed-off-by: Kay Ackermann 
---
hw/e1000.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 028afd1..3987e70 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -789,6 +789,8 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = {
getreg(MANC), getreg(MDIC), getreg(SWSM), getreg(STATUS),
getreg(TORL), getreg(TOTL), getreg(IMS), getreg(TCTL),
getreg(RDH), getreg(RDT), getreg(VET), getreg(ICS),
+ getreg(TDBAL), getreg(TDBAH), getreg(RDBAH), getreg(RDBAL),
+ getreg(TDLEN), getreg(RDLEN),

[TOTH] = mac_read_clr8, [TORH] = mac_read_clr8, [GPRC] = mac_read_clr4,
[GPTC] = mac_read_clr4, [TPR] = mac_read_clr4, [TPT] = mac_read_clr4,
-- 
1.6.4.4







Re: [Qemu-devel] Re: [PATCH V6 17/32] pci: 64bit bar support.

2009-11-04 Thread Michael S. Tsirkin
On Wed, Nov 04, 2009 at 03:20:02PM +0900, Isaku Yamahata wrote:
> On Tue, Nov 03, 2009 at 04:09:16PM +0200, Michael S. Tsirkin wrote:
> > > > Long term, we should fix all devices and *then* they can claim 64 bit
> > > > support always.  As a nice side effect, we'll be able to avoid
> > > > rebuilding devices.
> > > 
> > > Are you claiming that (PCI) devices emulation shouldn't depend on
> > > target_phys_addr_t? That sounds a good idea.
> > 
> > Yes. Maybe we can stop devices from mapping memory, have pci
> > core do it for them.
> 
> IIRC Avi proposed such a patch,

link?

> but it didn't get merged.

I wonder why - currently mapping is done by devices but unmapping is
done by pci core. That's been always bothering me.


> -- 
> yamahata




Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

2009-11-04 Thread Alexander Graf


On 04.11.2009, at 01:28, Anthony Liguori wrote:

This series solves a problem that I've been struggling with for a  
few years now.
One of the best things about qemu is that it's possible to run  
guests as an
unprivileged user to improve security.  However, if you want to have  
your guests
communicate with the outside world, you're pretty much forced to run  
qemu as

root.

At least with KVM support, this is probably the most common use case  
which means

that most of our users are running qemu as root.  That's terrible.


Yeah. Worse than the "run as root" part is the "it's hard" part  
though. I hate how I feel when I try to explain someone how to use non- 
slirp networking :-(.


The response to that is then usually "oh whatever, it's too  
complicated anyways".


We address this problem by introducing a new network backend: -net  
bridge.  This
backend is less flexible than -net tap because it relies on a helper  
with
elevated privileges to do the heavy lifting of allocating and  
attaching a tap
device to a bridge.  We use a special purpose helper because we  
don't want

to elevate the privileges of more generic tools like brctl.

From a user perspective, to use bridged networking with a guest, you  
simply use:


 qemu -hda linux.img -net bridge -net nic

And assuming a bridge is defined named qemubr0 and the administrator  
has setup
permissions accordingly, it will Just Work.  My hope is that  
distributions will
do this work as part of the qemu packaging process such that for  
most users,

the out-of-the-box experience will also Just Work.


Yeah, that won't work as easily.

When your customer has 2 NICs this will already break. But let's  
imagine we only have a single NIC.


So that NIC is a wifi card. When I set up the qemubr0 bridge for that  
one now, how does network manager configure my wifi access? It can't  
use the bridge device, as that doesn't have wifi extensions. It also  
can't use the wifi device, because setting up networking on that will  
break.


IMHO the only customer friendly choice I see is the ugly way. The way  
VMware and Vbox do it.
To make it a bit less ugly, maybe we could create some way to "glue" a  
tap device and an eth together, the same way the bridge code does,  
just without the extra device.



Alex




[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.

2009-11-04 Thread Alexander Graf


On 04.11.2009, at 07:14, Isaku Yamahata wrote:


On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote:

--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ##  
__VA_ARGS__); } while (0)

#define PCI_DPRINTF(fmt, ...)
#endif

+static void pci_host_config_writel(void *opaque,  
target_phys_addr_t addr,

+   uint32_t val)
+{
+PCIHostState *s = opaque;
+
+#ifdef TARGET_WORDS_BIGENDIAN
+val = bswap32(val);
+#endif


I know you just copied it, but isn't this just
val = le32_to_cpu(val);

?


Makes sense.



+static void pci_host_config_writel_noswap(void *opaque,
+  target_phys_addr_t addr,
+  uint32_t val)
+{
+PCIHostState *s = opaque;
+
+PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
+__func__, addr, val);
+s->config_reg = val;
+}
+
+static uint32_t pci_host_config_readl_noswap(void *opaque,
+ target_phys_addr_t  
addr)

+{
+PCIHostState *s = opaque;
+uint32_t val = s->config_reg;
+
+PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n",
+__func__, addr, val);
+return val;
+}
+
+static CPUWriteMemoryFunc * const pci_host_config_write_noswap[]  
= {

+&pci_host_config_writel_noswap,
+&pci_host_config_writel_noswap,
+&pci_host_config_writel_noswap,
+};
+
+static CPUReadMemoryFunc * const pci_host_config_read_noswap[] = {
+&pci_host_config_readl_noswap,
+&pci_host_config_readl_noswap,
+&pci_host_config_readl_noswap,
+};
+
+int pci_host_config_register_io_memory_noswap(PCIHostState *s)


This name is clearly too long,
as a result when you use it you run over the 80 character
limit. Let's not fix it, let's make name shorter.
io_memory -> memory?


+{
+return cpu_register_io_memory(pci_host_config_read_noswap,
+  pci_host_config_write_noswap, s);
+}


Do you happen to know whether no swap is really needed?  I suspect  
some
of these places really only happen to work because everyone uses  
intel,

so they simply would not work on ppc host.


I just followed the original code not to break it.
I dug into the commit logs a bit.
Liu, Aurelian and Alexander, could you give me a comment on
whether those functions needs byte swap (le32_to_cpu()) or not.
 - ppcie500_cfgaddr_{write, read}l() in ppce500_pci.c
 - pci_unin_config_{write, read}l() in unin_pci.c

ppce500_pci.c case:
The related commit is 74c62ba88902575be9ac3badf95d773470884b1c.
The comment on the top in the file says that it is derived from  
ppc4xx_pci.c.

ppc4xx_pci.c has byte swap, on the other hand ppce500_pci.c doesn't.
So I'm inclined to suspect that the byte swap was dropped accidently.
However I don't know its pci host bridge spec.


unin_pci.c case:
I don't know its spec neither.
The following changeset seems to remove the byte swap deliberately.
Alexander, could you please give us comment on it?


Phew - I frankly don't remember. Something broke because some bits  
were strangely mangled and the way it's not it worked more than it did  
before :-). The whole Uninorth thing is still utterly broken, so  
please don't take anything from there as example.


I do know that Hollis was working a lot about LE/BE issues on PPC in  
Qemu, so he's probably the best person to CC and ask here.



Alex





[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

2009-11-04 Thread Amit Shah
On (Wed) Nov 04 2009 [10:39:39], Jan Kiszka wrote:
> Amit Shah wrote:
> > On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote:
> >> Amit Shah wrote:
> >>> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
>  On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
> > Amit Shah wrote:
> >> The initial_reset sent to chardevs doesn't do much other than setting
> >> a bool to true. Char devices are interested in the open event and
> >> that gets sent whenever the device is opened.
> > Have you checked with the monitor in all use cases (dedicated & muxed
> > console, stdio & SDL console, etc.)? It was introduced once to fix a
>  I've checked with -monitor stdio, monitor in SDL and also chardevs using
>  unix sockets.
> 
>  I've not tried mux yet; I'll try that and report back. BTW if it ends up
>  not working with this patch, it'd be broken in the current master as
>  well.
> >>> I tried with:
> >>>
> >>> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
> >>>
> >>> The monitor prompt shows up as does the serial output.
> >>>
> >>> (btw I've also tried closing and opening char devs and that works fine
> >>> too)
> >> That sounds good. Then something must have changed since 2970a6c943, do
> >> you see what?
> > 
> > I think that depended on the resets being sent. I've now removed the
> > need for resets altogether.
> 
> No, this is in fact the reason why we no longer need it:
> 
>   9a1e948129 (Introduce contexts for asynchronous callbacks)
> 
> As the initial reset of the char device that is marked pending on open
> is now no longer consumed by the IDE initialization, we can actually
> drop the later regeneration via qemu_chr_initial_reset. I just hope this
> stays like it is...

I tested this even on a tree that doesn't have this patch.

I haven't really delved deep to see why this was added earlier -- the
commit log only says very little. Plus my testing with the current tree
works fine so I'm happy to mention these things in the commit log.

Amit




[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling

2009-11-04 Thread Jan Kiszka
Amit Shah wrote:
> On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote:
>> Amit Shah wrote:
>>> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote:
 On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote:
> Amit Shah wrote:
>> The initial_reset sent to chardevs doesn't do much other than setting
>> a bool to true. Char devices are interested in the open event and
>> that gets sent whenever the device is opened.
> Have you checked with the monitor in all use cases (dedicated & muxed
> console, stdio & SDL console, etc.)? It was introduced once to fix a
 I've checked with -monitor stdio, monitor in SDL and also chardevs using
 unix sockets.

 I've not tried mux yet; I'll try that and report back. BTW if it ends up
 not working with this patch, it'd be broken in the current master as
 well.
>>> I tried with:
>>>
>>> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux
>>>
>>> The monitor prompt shows up as does the serial output.
>>>
>>> (btw I've also tried closing and opening char devs and that works fine
>>> too)
>> That sounds good. Then something must have changed since 2970a6c943, do
>> you see what?
> 
> I think that depended on the resets being sent. I've now removed the
> need for resets altogether.

No, this is in fact the reason why we no longer need it:

9a1e948129 (Introduce contexts for asynchronous callbacks)

As the initial reset of the char device that is marked pending on open
is now no longer consumed by the IDE initialization, we can actually
drop the later regeneration via qemu_chr_initial_reset. I just hope this
stays like it is...

Jan



signature.asc
Description: OpenPGP digital signature