headers
Markus Armbruster | 1 Jul 2009 02:12
Picon
Favicon

Re: [PATCH 4/8] qdev/core: bus list

Anthony Liguori <anthony <at> codemonkey.ws> writes:

> Markus Armbruster wrote:
>> Paul Brook <paul <at> codesourcery.com> writes:
>>
>>   
>>> On Tuesday 30 June 2009, Markus Armbruster wrote:
>>>     
>>>> Paul Brook <paul <at> codesourcery.com> writes:
>>>>       
>>>>> On Tuesday 30 June 2009, Gerd Hoffmann wrote:
>>>>>         
>>>>>>  * maintain a list of busses.
>>>>>>  * maintain bus numbers.
>>>>>>  * add function to find busses by type / name / number.
>>>>>>  * add monitor command to list busses.
>>>>>>           
>>>>> I still object to this patch. Busses should be identified by their
>>>>> location in the tree, not by number.
>>>>>
>>>>> Paul
>>>>>         
>>>> Location in the tree can be uniquely identified by a number.  Handy when
>>>> all you want is enumerate the buses, and you don't really care where
>>>> they're hanging out in the tree.  Why should something like that not be
>>>> done?
>>>>       
>>> The address of the BusState is also a locally unique
>>> identifier. That doesn't mean it's a good thing to expose to the
>>> user.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Paul Brook | 1 Jul 2009 02:25
Gravatar

Re: [PATCH 4/8] qdev/core: bus list

On Tuesday 30 June 2009, Gerd Hoffmann wrote:
> On 06/30/09 21:49, Anthony Liguori wrote:
> > I think it's a perfectly valid suggestion that we should identify buses
> > based on the their location in the tree to users verses a number
> > generated based on some hashing algorithm.
> >
> > A tree location has meaning to a user. A random integer doesn't.
>
> Well.  Depends on the bus I think.  About PCI devices the usual user
> probably thinks in terms of "$bus:$slot.$function", which includes a bus
> number.
>
> Speaking of PCI: the PCI bus number (aka PCIBus->bus_num) has nothing to
> do with the more or less random bus number introduced by the (now
> dropped) patch (aka PCIBus->qdev.busnr).  Which indicates that it is
> probably less confusing to have the bus implementation handle the
> enumeration of busses.  If it makes sense for the bus in question of
> course.  sysbus probably doesn't care ;)

PCI bus numbers should be determined by the guest, and are not fixed values. 
The only reason we get away with it now is because most guests will honor a 
pre-existing bridge device configuration. A guest firmware could choose to 
enumerate the PCI busses in a different order.

Paul
Permalink | Reply | 1 comment |
headers
Paul Brook | 1 Jul 2009 02:29
Gravatar

Re: [PATCH 4/8] qdev/core: bus list

> >>> The address of the BusState is also a locally unique
> >>> identifier. That doesn't mean it's a good thing to expose to the
> >>> user.
> >>
> >> Red herring.
> >
> > I don't think that's a very useful response.
> >
> > I think it's a perfectly valid suggestion that we should identify
> > buses based on the their location in the tree to users verses a number
> > generated based on some hashing algorithm.
> >
> > A tree location has meaning to a user.  A random integer doesn't.
>
> Numbering nodes according to a well-defined tree traversal is not
> random.  We can discuss whether using such a number in an interface is a
> good idea (nobody suggested to use it *instead* of tree paths).

I don't believe the tree traversal order is well defined. While developing the 
qdev patches I went through two or three different traversal algorithms. It 
gets even hairier when you start considering hotplug.

Paul
Permalink | Reply | 0 comments |
headers
Huang Ying | 1 Jul 2009 03:14
Picon
Favicon

Re: [PATCH -v6] QEMU: MCE: Add MCE simulation to qemu/tcg

Hi, Anthony,

On Wed, 2009-07-01 at 02:12 +0800, Anthony Liguori wrote:
> Hi Huang,
> 
> Huang Ying wrote:
> > - MCE features are initialized when VCPU is intialized according to CPUID.
> > - A monitor command "mce" is added to inject a MCE.
> > - A new interrupt mask: CPU_INTERRUPT_MCE is added to inject the MCE.
> >   
> 
> Can you post some instructions on how to test this functionality?

During development, I use QEMU monitor command "mce" to test this
functionality. For corrected error, you can use /sbin/mcelog to get the
error information. For uncorrected error, you will get panic. If you
want to get result faster for corrected error, set the following sysfs
file to a small number, which is seconds between corrected error polls.

/sys/devices/system/machinecheck/machinecheck0/check_interval

For corrected error:

mce 0 1 0x8000000000000000 0x0 0x0 0x0

For uncorrected error:

mce 0 1 0xb200000000000000 0x0 0x0 0x0

> For
(Continue reading)

Permalink | Reply | 0 comments |
headers
Gerd Hoffmann | 1 Jul 2009 08:32
Picon
Favicon

Re: [PATCH 4/8] qdev/core: bus list

> PCI bus numbers should be determined by the guest, and are not fixed values.

They are not fixed.  Guest writes to the bridge registers will update 
PCIBus->bus_num, so the guests and qemus view to pci bus numbers should 
match even if the guest changes them.

cheers,
   Gerd
Permalink | Reply | 0 comments |
headers
Avi Kivity | 1 Jul 2009 10:07
Picon
Favicon

Re: [PATCH 01/11] QMP: Introduce specification file

On 06/30/2009 09:21 PM, Anthony Liguori wrote:
>> 1. Add QValue, QArray, QDictionary
>>
>> QValue is a polymorphic object that can be a number, a string, an 
>> array, or a dictionary.  It needs a bunch of accessors like 
>> qvalue_to_qarray() to check if a value is an array, and things like 
>> qarray_get() or qdict_put() to manipulate them.  For the simpler data 
>> types, qvalue_to_int64() and qvalue_from_int64() should suffice.
>
>
> And add a return value to the monitor functions.  I think this is a 
> good first step.
>
> Passing a QArray to each monitor function is one way to go.  Another 
> way to go is to use something like libffi to do proper dynamic 
> dispatch.  The later approach has the benefit of requiring less code 
> churn and reusing the static type checking.

If/when we switch to passing dictionaries instead of arrays, this won't 
work.  I also think libffi is a little to exotic for something like the 
monitor.
You could potentially introduce some serious ninja action by doing 
something like:

> value = qvalue_create("[%s, {'f': %s, 'b': %s', 'c': %d}]", ...);

Seriously nifty...

--

-- 
error compiling committee.c: too many arguments to function
(Continue reading)

Permalink | Reply | 0 comments |
headers
Kevin Wolf | 1 Jul 2009 09:37
Picon
Favicon

Re: [PATCH] block-raw: Allow pread beyond the end of growable images

Christoph Hellwig schrieb:
> On Fri, Jun 26, 2009 at 07:51:24PM +0200, Kevin Wolf wrote:
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index fa1a394..985bf69 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>>  <at>  <at>  -117,6 +117,7  <at>  <at>  typedef struct BDRVRawState {
>>  static int posix_aio_init(void);
>>  
>>  static int fd_open(BlockDriverState *bs);
>> +static int64_t raw_getlength(BlockDriverState *bs);
>>  
>>  #if defined(__FreeBSD__)
>>  static int cdrom_reopen(BlockDriverState *bs);
>>  <at>  <at>  -231,6 +232,16  <at>  <at>  static int raw_pread_aligned(BlockDriverState *bs, int64_t offset,
>>      if (ret == count)
>>          goto label__raw_read__success;
>>  
>> +    /* Allow reads beyond the end (needed for pwrite) */
>> +    if ((ret == 0) && bs->growable) {
>> +        int64_t size = raw_getlength(bs);
>> +        if (offset >= size) {
>> +            memset(buf, 0, count);
>> +            ret = count;
>> +            goto label__raw_read__success;
>> +        }
>> +    }
> 
> I really don't like doing this inside the lowelevel read handler.  If
> this is indeed only needed for pwrite we might be better off doing
(Continue reading)

Permalink | Reply | 3 comments |
headers
Kevin Wolf | 1 Jul 2009 10:35
Picon
Favicon

Re: [COMMIT ff24bd5] qemu/virtio: virtio save/load bindings

Anthony Liguori schrieb:
> From: Michael S. Tsirkin <mst <at> redhat.com>
> 
> Implement bindings for virtio save/load. Use them in virtio pci.
> 
> Signed-off-by: Michael S. Tsirkin <mst <at> redhat.com>
> Signed-off-by: Anthony Liguori <aliguori <at> us.ibm.com>

This patch causes segfaults during savevm:

Program received signal SIGSEGV, Segmentation fault.
0x0000003d1d0842a2 in memcpy () from /lib64/libc.so.6
(gdb) bt
#0  0x0000003d1d0842a2 in memcpy () from /lib64/libc.so.6
#1  0x00000000004a65bf in qemu_put_buffer (f=0xd4f220, buf=0x0, size=48)
at savevm.c:478
#2  0x0000000000419133 in msix_save (dev=0xc93410, f=0xd4f220) at
/home/kwolf/source/qemu/hw/msix.c:289
#3  0x0000000000555424 in virtio_pci_save_config (opaque=0xc93410,
f=0xd4f220) at /home/kwolf/source/qemu/hw/virtio-pci.c:112
#4  0x00000000005545b3 in virtio_save (vdev=0xc93ad0, f=0xd4f220) at
/home/kwolf/source/qemu/hw/virtio.c:620
#5  0x0000000000419eaf in virtio_balloon_save (f=0xd4f220,
opaque=0xc93ad0) at /home/kwolf/source/qemu/hw/virtio-balloon.c:151
#6  0x00000000004a6756 in qemu_savevm_state_complete (f=0xd4f220) at
savevm.c:811
#7  0x00000000004a68dd in qemu_savevm_state (f=0xd4f220) at savevm.c:842
#8  0x00000000004a6a4b in do_savevm (mon=0xcd87b0, name=<value optimized
out>) at savevm.c:1134
#9  0x00000000004138f4 in monitor_handle_command (mon=0xcd87b0,
(Continue reading)

Permalink | Reply | 3 comments |
headers
Christoph Hellwig | 1 Jul 2009 10:43
Picon

Re: [PATCH] ATAPI pass through

On Tue, Jun 30, 2009 at 06:55:42PM +0100, Bique Alexandre wrote:
> I can switch to aio_ioctl, and use CDROM_SEND_PACKET ioctl.

CDROM_SEND_PACKET has a rather horrible API and should be avoided.
Just use SG_IO like the the scsi passthrough code.  In fact there might
be some quite a few ways to unify that code if you're motivated.

> Is it possible to send multiple requests at the same time with aio_ioctl ? 

SG_IO sends one requests per ioctl, but you can have multiple requests
outstanding on a single file descriptor.

> When CONFIG_AIO is not available ?

aio_ioctl needs CONFIG_AIO - but on Linux hosts where you have
bsd/sg/etc it always is available.
Permalink | Reply | 0 comments |
headers
Christoph Hellwig | 1 Jul 2009 10:44
Picon

Re: [PATCH] ATAPI pass through

On Wed, Jul 01, 2009 at 08:57:37AM +0200, Alexander Graf wrote:
> But last time I checked, we had a cdrom emulation in scsi-disk.c _and_  
> in ide.c.
> 
> I personally see no reason for that. Why can't we just have an atapi  
> emulation layer in ide.c and use whatever scsi backends we have? That  
> way, we'd only need to implement cd-rom and scsi passthrough once and  
> have it available either via SCSI or via ATAPI.

That would be best.  Note that we don't implement SCSI passthrough in
scsi-disk.c but rather in scsi-generic.c.  But being able to attach
both of them to ata.c using an ATAPI shim would indeed be best.
Permalink | Reply | 0 comments |

Gmane