Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Ian Jackson]

Gerd Hoffmann writes ("[Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
> Here are a bunch of patches which start adding xen support to qemu.
> Overview (individual patches have longer descriptions):

Just to clarify: as far as I can tell from the description,
this code has scant relationship with Xen upstream.

I'm generally in favour of pushing functionality out of the Xen branch
of qemu into upstream.  But going by Gerd Hoffman's description I
don't think that's what his branch is.  His code doesn't appear to be
related to the Xen branch of qemu that we are using.

For example,

> With the first four patches in place upstream qemu can replace xen's
> qemu-dm for paravirtual domains.  The block and nic backend drivers are
> full userspace implementations using the grant table device (gntdev).

we only use qemu-dm in paravirtualised domains for certain marginal
functions - in many cases it is not used at all.  Certainly the
functionality Gerd describes, such as xen backend block and network
drivers, are not in our qemu tree and we do not intend for them to be
there.

In a Xen installation this functionality is in the dom0 (host) kernel.


As far as I can see much of Gerd Hoffman's intended submission is
effectively an _emulator_ for Xen guests.  That is, it emulates a Xen
host without being one, so that a Xen domU can be run without the Xen
hypervisor.  Am I right, Gerd ?

I don't think there's anything wrong with that from a qemu point of
view.  Obviously we think that the genuine Xen hypervisor is much
better :-) but it is right for people to have a choice, and having
qemu emulate more environments is generally good.

But if this functionality is to go into qemu upstream perhaps it
should be distinguished from `real Xen' functions, because otherwise
users are going to become very very confused.

Ian.

Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Anthony Liguori]

Ian Jackson wrote:
> Gerd Hoffmann writes ("[Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
>   
>> Here are a bunch of patches which start adding xen support to qemu.
>> Overview (individual patches have longer descriptions):
>>     
>
> Just to clarify: as far as I can tell from the description,
> this code has scant relationship with Xen upstream.
>
> I'm generally in favour of pushing functionality out of the Xen branch
> of qemu into upstream.  But going by Gerd Hoffman's description I
> don't think that's what his branch is.  His code doesn't appear to be
> related to the Xen branch of qemu that we are using.
>   

I think it's more closely related to Xenite and Xenner.  Gerd: are you 
planning on folding in domain creation?  Right now it appears to be a 
helper launched after the domain creation.

> For example,
>
>   
>> With the first four patches in place upstream qemu can replace xen's
>> qemu-dm for paravirtual domains.  The block and nic backend drivers are
>> full userspace implementations using the grant table device (gntdev).
>>     
>
> we only use qemu-dm in paravirtualised domains for certain marginal
> functions - in many cases it is not used at all.  Certainly the
> functionality Gerd describes, such as xen backend block and network
> drivers, are not in our qemu tree and we do not intend for them to be
> there.
>   

There's no reason they couldn't be though.  It's just like blktap.

> In a Xen installation this functionality is in the dom0 (host) kernel.
>
>
> As far as I can see much of Gerd Hoffman's intended submission is
> effectively an _emulator_ for Xen guests.  That is, it emulates a Xen
> host without being one, so that a Xen domU can be run without the Xen
> hypervisor.  Am I right, Gerd ?
>   

No, it's definitely for use with Xen (hypervisor).  But it's different 
architecturally from how Xen uses QEMU in xen-unstable.

Personally, I really like the way Gerd has the code structured.  It 
seems like it could be used as almost a drop-in replacement for qemu-dm 
for PV guests.  HVM guests would require more work.  Of course, the 
net/disk support can be completely optionally.

With that said, it would be silly to have two Xen implementations within 
QEMU so there has to be some general agreement in the Xen community 
about how QEMU is going to be used before merging would make sense.  
That probably requires some discussion about how the HVM support would 
fit into this architecture.

Regards,

Anthony Liguori



> I don't think there's anything wrong with that from a qemu point of
> view.  Obviously we think that the genuine Xen hypervisor is much
> better :-) but it is right for people to have a choice, and having
> qemu emulate more environments is generally good.
>
> But if this functionality is to go into qemu upstream perhaps it
> should be distinguished from `real Xen' functions, because otherwise
> users are going to become very very confused.
>
> Ian.
>
>
>   

Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Ian Jackson wrote:
> Gerd Hoffmann writes ("[Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
>> Here are a bunch of patches which start adding xen support to qemu.
>> Overview (individual patches have longer descriptions):
> 
> Just to clarify: as far as I can tell from the description,
> this code has scant relationship with Xen upstream.

Patches #3 + #4 are largely based on qemu-dm code (as noted in the
individual patch  descriptions).

> I'm generally in favour of pushing functionality out of the Xen branch
> of qemu into upstream.  But going by Gerd Hoffman's description I
> don't think that's what his branch is.  His code doesn't appear to be
> related to the Xen branch of qemu that we are using.

I want to merge the *functionality*.  IMHO that doesn't mean that it
must be the *code* used by Xen at the moment.

> For example,
> 
>> With the first four patches in place upstream qemu can replace xen's
>> qemu-dm for paravirtual domains.  The block and nic backend drivers are
>> full userspace implementations using the grant table device (gntdev).
> 
> we only use qemu-dm in paravirtualised domains for certain marginal
> functions - in many cases it is not used at all.

It's used for xen console (optionally, can also be handled by
xenconsoled) and the virtual framebuffer.

> Certainly the
> functionality Gerd describes, such as xen backend block and network
> drivers, are not in our qemu tree and we do not intend for them to be
> there.

I want them be there though.  You can use them or not, that is your call.

> In a Xen installation this functionality is in the dom0 (host) kernel.

That is only half the story.  The block backend can run in userspace too
(when using blktap).  The block backend driver should be pretty much
identical to blktap functionality-wise.  The implementation is quite
different though.

> As far as I can see much of Gerd Hoffman's intended submission is
> effectively an _emulator_ for Xen guests.  That is, it emulates a Xen
> host without being one, so that a Xen domU can be run without the Xen
> hypervisor.  Am I right, Gerd ?

That is part of the longterm plan, yes.  I want qemu being able to do
both, run as device model for xen and also to boot xen guest images
without xen, using emulation.  If the intention would have been
emulation only I wouldn't have Cc'ed xen-devel for patch review.

The emulation bits are not in that patchset btw.

> But if this functionality is to go into qemu upstream perhaps it
> should be distinguished from `real Xen' functions, because otherwise
> users are going to become very very confused.

Huh?

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Ian Jackson]

Anthony Liguori writes ("Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
> I think it's more closely related to Xenite and Xenner.  Gerd: are you 
> planning on folding in domain creation?  Right now it appears to be a 
> helper launched after the domain creation.
...
> No, it's definitely for use with Xen (hypervisor).  But it's different 
> architecturally from how Xen uses QEMU in xen-unstable.

Xenner is an emulator for allowing Xen domUs to be booted without the
Xen hypervisor.

Xennite is an experimental replacement for the Xen userland management
stack in dom0: it moves more functionality from the Xen tools in dom0
into the qemu-dm process.  This is moving in almost the opposite
direction to Xen upstream is moving: we are moving qemu-dm into its
own tiny domain, so that the qemu code doesn't need to run as a
process in dom0; this has important security and scalability
advantages.

Ian.

Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Anthony Liguori wrote:
> I think it's more closely related to Xenite and Xenner.  Gerd: are you
> planning on folding in domain creation?  Right now it appears to be a
> helper launched after the domain creation.

Not sure yet.  Given that xen upstream most likely wouldn't follow a
xenite-like model I'm not sure how useful it would be.  Shouldn't be
that much extra code though, and as long as it is an opt-in upstream xen
should be happy too ...

> It
> seems like it could be used as almost a drop-in replacement for qemu-dm
> for PV guests.

Yes, that is the intention.

> HVM guests would require more work.

Yes.  Not addressed (yet).  hvm is more code, more invasive and I also
hope that Glauber Costa's work on a cpu abstraction layer (for
emulation/kqemu/kvm) makes it easier to add xen hvm to the picture.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Anthony Liguori]

Ian Jackson wrote:
> Anthony Liguori writes ("Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
>   
>> I think it's more closely related to Xenite and Xenner.  Gerd: are you 
>> planning on folding in domain creation?  Right now it appears to be a 
>> helper launched after the domain creation.
>>     
> ...
>   
>> No, it's definitely for use with Xen (hypervisor).  But it's different 
>> architecturally from how Xen uses QEMU in xen-unstable.
>>     
>
> Xenner is an emulator for allowing Xen domUs to be booted without the
> Xen hypervisor.
>   

Or "shim".  It's almost architecturally identical to the XenSource 
developed "shim" for Hyper-V.  It seems like a popular thing to do these 
days :-)

> Xennite is an experimental replacement for the Xen userland management
> stack in dom0: it moves more functionality from the Xen tools in dom0
> into the qemu-dm process.  This is moving in almost the opposite
> direction to Xen upstream is moving: we are moving qemu-dm into its
> own tiny domain, so that the qemu code doesn't need to run as a
> process in dom0; this has important security and scalability
> advantages.
>   

Are there separate requirements for stub domains verses Xennite?

Gerd's code is different than what's upstream in QEMU but the question 
is whether it's reconcilable with what's upstream.  If not, what makes 
it that way?

Regards,

Anthony Liguori

> Ian.
>   

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Mon 28 Jul 2008 16:43:54 +0200, a écrit :
> Ian Jackson wrote:
> > I'm generally in favour of pushing functionality out of the Xen branch
> > of qemu into upstream.  But going by Gerd Hoffman's description I
> > don't think that's what his branch is.  His code doesn't appear to be
> > related to the Xen branch of qemu that we are using.
> 
> I want to merge the *functionality*.  IMHO that doesn't mean that it
> must be the *code* used by Xen at the moment.

Well, the patch comments weren't so clear about that.

Samuel

Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Daniel P. Berrange]

On Mon, Jul 28, 2008 at 03:58:15PM +0100, Ian Jackson wrote:
> Anthony Liguori writes ("Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
> > I think it's more closely related to Xenite and Xenner.  Gerd: are you 
> > planning on folding in domain creation?  Right now it appears to be a 
> > helper launched after the domain creation.
> ...
> > No, it's definitely for use with Xen (hypervisor).  But it's different 
> > architecturally from how Xen uses QEMU in xen-unstable.
> 
> Xenner is an emulator for allowing Xen domUs to be booted without the
> Xen hypervisor.
> 
> Xennite is an experimental replacement for the Xen userland management
> stack in dom0: it moves more functionality from the Xen tools in dom0
> into the qemu-dm process.  This is moving in almost the opposite
> direction to Xen upstream is moving: we are moving qemu-dm into its
> own tiny domain, so that the qemu code doesn't need to run as a
> process in dom0; this has important security and scalability
> advantages.

Yes, to be clear that the Xennite code is *not* an official Xen project. 

It is my experimental work and I don't expect anyone to use it for real
in the near future. It does however align very well with Gerd's serious 
Xenner project (which is incredibly useful tool allowing admins to mix
and match Xen and KVM) since both Xenner & Xennite have much same needs
in  terms of QEMU support code

The motiviation with 'Xennite' is to make the process of launching
and managing an instance of a Xen guest, as close as possible to that
of KVM. Start QEMU to launch the Xen guest, interact with the monitor
to control it, and kill QEMU to destroy the Xen guest. I'd class it as
a proof-of-concept since I never got around to making stuff like save
and restore work - I postponed further work until Xen re-synced with a
new QEMU codebase.  We could have a long discussion about the pros/cons
of this idea vs the upstream Xen move to QEMU in stub domains, but that's
off topic for this list, so I'll leave it :-)  

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Markus Armbruster]

Samuel Thibault <samuel.thibault@eu.citrix.com> writes:

> Gerd Hoffmann, le Wed 06 Aug 2008 12:14:25 +0200, a écrit :
>> The rough way to merge would look like this:
>> 
>>   - drop xen_console.[ch]
>>   - drop xenfb.[ch]
>>   - drop xen_machine_pv.c
>> 
>>   - add xen.h
>>   - add xen-machine.c
>>   - add xen-backend.[ch]
>>   - add xen-console.c
>>   - add xen-framebuffer.c
>> 
>>   - wind up stuff in the Makefiles.
>>   - some global renames (domid -> xen_domid for example) as I took care
>>     to prefix global xen variables & functions with xen_.
>>   - probably some small fixups are needed ...
>
> You forgot the _test_ stage.  You are basically asking us to replace our
> well-tested implementation with your implementation, that's quite a
> move.  You are not even providing a patch for us to check that nothing
> has been left behind...

It *is* quite move, becauses it accomplishes a lot: it goes from a
heavily modified fork of an oldish version all the way to merge with
upstream, as far as PV is concerned.

If that's where we want to go, we can of course still argue whether we
should go in leaps or baby steps, and whether Gerd's leap lands in
quite the right spot.  But the distance to conquer remains the same,
and so does the testing challenge.

For what it's worth, I went over significant parts of Gerd's patch
(all the generic stuff + pvfb) with a fine comb, comparing it to what
we have now.  I consider it sound.

[...]

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Markus Armbruster, le Wed 06 Aug 2008 08:43:49 -0400, a écrit :
> It *is* quite move, becauses it accomplishes a lot: it goes from a
> heavily modified fork of an oldish version all the way to merge with
> upstream, as far as PV is concerned.

Then why doing it in qemu before having it tested in the xen unstable
tree?  That's not the way I usually see merging happen.

> For what it's worth, I went over significant parts of Gerd's patch
> (all the generic stuff + pvfb) with a fine comb, comparing it to what
> we have now.  I consider it sound.

I'm not saying it's not fine.  I had a look and the code looked fine
indeed.  But what I'm afraid of is the delta with Ian would have to
bear when merging: is it save/restore safe, does it work with PCI
pass-through, VT-D, etc.?

> If that's where we want to go, we can of course still argue whether we
> should go in leaps or baby steps, and whether Gerd's leap lands in
> quite the right spot.

Baby steps are much easier to review.  That's how things are usually
done, and here it looks to me like it is feasible to achieve in Xen (and
have it tested) before event thinking about importing a pile of code in
qemu where it won't receive as much testing as the xen-unstable tree
receives.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

  Hi,

> Baby steps are much easier to review.  That's how things are usually
> done, and here it looks to me like it is feasible to achieve in Xen (and
> have it tested) before event thinking about importing a pile of code in
> qemu where it won't receive as much testing as the xen-unstable tree
> receives.

IMHO the best way to address this issue is to rebase frequently.  The
xen-unstable testing will also covers qemu then.  It also keeps the
delta smaller and makes it easier to test qemu patches in qemu-dm.

I'm trying to fiddle my bits into qemu-dm right now.  One problem is
that I get build failures simply because of the fact that the qemu-dm
base is old compared to upstream qemu.

It would also be great if you can clean up the header mess.  qemu-dm
doesn't build without xen-unstable.hg.  And just a checked out tree
isn't good enougth, it must be compiled because qemu-dm depends on some
generated header files.  Aiee.  IMHO qemu-dm should only need
/usr/include/xen.  If headers are missing there, they should be installed.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
> IMHO the best way to address this issue is to rebase frequently.

Then why not pushing the changes to xen repositories in the first place?

> I'm trying to fiddle my bits into qemu-dm right now.  One problem is
> that I get build failures simply because of the fact that the qemu-dm
> base is old compared to upstream qemu.

Use Ian Jackson's tree!

> It would also be great if you can clean up the header mess.  qemu-dm
> doesn't build without xen-unstable.hg.  And just a checked out tree
> isn't good enougth, it must be compiled because qemu-dm depends on some
> generated header files.  Aiee.  IMHO qemu-dm should only need
> /usr/include/xen.  If headers are missing there, they should be installed.

You could submit patches...

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Samuel Thibault wrote:
> Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
>> IMHO the best way to address this issue is to rebase frequently.
> 
> Then why not pushing the changes to xen repositories in the first place?

Because I want have xen support in *upstream* qemu.

>> I'm trying to fiddle my bits into qemu-dm right now.  One problem is
>> that I get build failures simply because of the fact that the qemu-dm
>> base is old compared to upstream qemu.
> 
> Use Ian Jackson's tree!

Adapt my patches to an oldish qemu fork, so they must be adapted again
for submitting upstream?  No, thank you.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Wed 06 Aug 2008 16:08:47 +0200, a écrit :
> Samuel Thibault wrote:
> > Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
> >> IMHO the best way to address this issue is to rebase frequently.
> > 
> > Then why not pushing the changes to xen repositories in the first place?
> 
> Because I want have xen support in *upstream* qemu.

I know, but you are basically saying "ok, guys, you'll just manage the
breakage of the big merge, thanks", that's not very convenient, to say
the least.  For instance, where should fix patches be sent to?  Both
qemu-devel and xen-devel?  Just xen-devel and thus make Ian have to push
them to qemu-devel?

Pushing the cleaning changes to Xen first can be done and would entail
much easier to tackle breakage, and the merge back from qemu would then
be trivial, why not doing so?

> >> I'm trying to fiddle my bits into qemu-dm right now.  One problem is
> >> that I get build failures simply because of the fact that the qemu-dm
> >> base is old compared to upstream qemu.
> > 
> > Use Ian Jackson's tree!
> 
> Adapt my patches to an oldish qemu fork, so they must be adapted again
> for submitting upstream?  No, thank you.

AGAIN, THERE IS THE IAN JACKSON TREE.

Thanks for reading.  Ian has _already_ done that hard work.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Samuel Thibault wrote:
> Gerd Hoffmann, le Wed 06 Aug 2008 16:08:47 +0200, a écrit :
>> Samuel Thibault wrote:
>>> Gerd Hoffmann, le Wed 06 Aug 2008 15:54:33 +0200, a écrit :
>>>> IMHO the best way to address this issue is to rebase frequently.
>>> Then why not pushing the changes to xen repositories in the first place?
>> Because I want have xen support in *upstream* qemu.
> 
> I know, but you are basically saying "ok, guys, you'll just manage the
> breakage of the big merge, thanks", that's not very convenient, to say
> the least.  For instance, where should fix patches be sent to?  Both
> qemu-devel and xen-devel?  Just xen-devel and thus make Ian have to push
> them to qemu-devel?

Most of the patches probably to both qemu-devel and xen-devel.  Stuff
unrelated to xen (say e1000 emulation bugs) qemu-devel only.

> Pushing the cleaning changes to Xen first can be done and would entail
> much easier to tackle breakage, and the merge back from qemu would then
> be trivial, why not doing so?
> 
>>>> I'm trying to fiddle my bits into qemu-dm right now.  One problem is
>>>> that I get build failures simply because of the fact that the qemu-dm
>>>> base is old compared to upstream qemu.
>>> Use Ian Jackson's tree!
>> Adapt my patches to an oldish qemu fork, so they must be adapted again
>> for submitting upstream?  No, thank you.
> 
> AGAIN, THERE IS THE IAN JACKSON TREE.
>
> Thanks for reading.

I get build failures when patching THE IAN JACKSON TREE due to THE IAN
JACKSON TREE lagging behind upstream.

I can read, thank you.  Please stop crying.

> Ian has _already_ done that hard work.

Oh, rebasing isn't a one-shot thing.  You have to do it over and over
again.  Unless you get your code upstream of course.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Wed 06 Aug 2008 17:35:39 +0200, a écrit :
> > AGAIN, THERE IS THE IAN JACKSON TREE.
> >
> > Thanks for reading.
> 
> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
> JACKSON TREE lagging behind upstream.

You didn't mention that. "lagging" a few months indeed, while merging
stuff.  And because xen 4.0 is about to be released, but it will
probably catch up soon. Anyway that's not a reason for not using it, I
guess the build failures can be fixed more easily than redoing Ian's
work.

> > Ian has _already_ done that hard work.
> 
> Oh, rebasing isn't a one-shot thing.  You have to do it over and over
> again.

Sure, that's why Ian was quite harsh on the modifications to the qemu
code, and these are quite small now (I'm talking about the Xen-specific
things, non-Xen-specific will be pushed upstream), so that shouldn't be
a problem any more.

SAmuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Laurent Vivier]

Le mercredi 06 août 2008 à 17:35 +0200, Gerd Hoffmann a écrit :
> Samuel Thibault wrote:
[...]
> > AGAIN, THERE IS THE IAN JACKSON TREE.
> >
> > Thanks for reading.
> 
> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
> JACKSON TREE lagging behind upstream.
> 
> I can read, thank you.  Please stop crying.


Hi guys,

if you want to merge your "xen-bits" into qemu, I think you should work
together not against each other...

(whereas personally I prefer KVM ;-) ) 

Gerd, IMHO, starting from scratch is never the best solution.

Samuel, perhaps you can help Gerd...

Regards,
Laurent
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Samuel Thibault wrote:
> Gerd Hoffmann, le Wed 06 Aug 2008 17:35:39 +0200, a écrit :
>>> AGAIN, THERE IS THE IAN JACKSON TREE.
>>>
>>> Thanks for reading.
>> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
>> JACKSON TREE lagging behind upstream.
> 
> You didn't mention that. "lagging" a few months indeed, while merging
> stuff.  And because xen 4.0 is about to be released, but it will
> probably catch up soon.

Well, for 4.0.x you probably have to branch off anyway, so the release
shouldn't stop catching up with upstream.

> Anyway that's not a reason for not using it, I
> guess the build failures can be fixed more easily than redoing Ian's
> work.

I'm using upstream qemu for development, and I'm not going to change that.

I can adapt the patches for the xen tree.  Having the xen tree *not*
lagging behind would be *very* helpful for keeping the trees and patches
in sync.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
> Pushing the cleaning changes to Xen first can be done and would entail
> much easier to tackle breakage, and the merge back from qemu would then
> be trivial, why not doing so?

You didn't answer that part.  Really, my only concern is about having
things tested.  Isn't it possible for instance to just merge the backend
core (and console/xenfb updates) in Ian's tree first for a start?  As
Markus and I said, it looked sane.  Then you can push your code to qemu,
I guess that could be fine, as you said xen will not need to use e.g.
the block and net backends.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Thu 07 Aug 2008 00:10:12 +0200, a écrit :
> Samuel Thibault wrote:
> > Gerd Hoffmann, le Wed 06 Aug 2008 17:35:39 +0200, a écrit :
> >>> AGAIN, THERE IS THE IAN JACKSON TREE.
> >>>
> >>> Thanks for reading.
> >> I get build failures when patching THE IAN JACKSON TREE due to THE IAN
> >> JACKSON TREE lagging behind upstream.
> > 
> > You didn't mention that. "lagging" a few months indeed, while merging
> > stuff.  And because xen 4.0 is about to be released, but it will
> > probably catch up soon.
> 
> Well, for 4.0.x you probably have to branch off anyway,

When 4.0 gets released, yes.

> so the release shouldn't stop catching up with upstream.

The way Xen has been working up to now is to branch the trunk just after
the release, not before.

> > Anyway that's not a reason for not using it, I
> > guess the build failures can be fixed more easily than redoing Ian's
> > work.
> 
> I'm using upstream qemu for development, and I'm not going to change that.

No problem.

> I can adapt the patches for the xen tree.

That's what I'm asking you from the start of the thread, I guess my
english is not so clear.

> Having the xen tree *not* lagging behind would be *very* helpful for
> keeping the trees and patches in sync.

Sure, it just happens that it couldn't be done before we get Ian's tree
working.  Once the 4.0 release is done we can tighten the gap.  And
things that shouldn't be only in Ian's tree (e.g. the enhanced serial
support) are being pushed already.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Samuel Thibault wrote:
> Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
>> Pushing the cleaning changes to Xen first can be done and would entail
>> much easier to tackle breakage, and the merge back from qemu would then
>> be trivial, why not doing so?
> 
> You didn't answer that part.  Really, my only concern is about having
> things tested.  Isn't it possible for instance to just merge the backend
> core (and console/xenfb updates) in Ian's tree first for a start?

http://kraxel.fedorapeople.org/patches/qemu-xen/

I didn't touch the build system, it is even more scary than the qemu one
alone, I just set CONFIG_XEN unconditionally.

I also largely left vl.c as-is, so xend shouldn't need any changes.  The
-domid switch sets an additional (redundant) variable, to keep the
amount of changes as small as possible for now.  Also -name and
-domain-name are aliased, both set qemu_name and domain_name.

In upstream qemu xenpv support is a runtime-switch for the normal qemu,
the xen patches leave the qemu-dm target in place.

The framebuffer driver probably has some performance regressions.
Fixing those depends on the display patches being pushed upstream.

> Then you can push your code to qemu,
> I guess that could be fine, as you said xen will not need to use e.g.
> the block and net backends.

blk and net backends are not there (yet).  But they should be a nop for
xen anyway as long as you don't wind up stuff in xend to put them in
use.  For the net backend it probably wouldn't be that useful.  The
block backend should be a good replacement for blktap though and maybe
can save you the effort of porting the blktap kernel driver to the
pv_ops kernel.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Thu 07 Aug 2008 09:33:06 +0200, a écrit :
> Samuel Thibault wrote:
> > Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
> >> Pushing the cleaning changes to Xen first can be done and would entail
> >> much easier to tackle breakage, and the merge back from qemu would then
> >> be trivial, why not doing so?
> > 
> > You didn't answer that part.  Really, my only concern is about having
> > things tested.  Isn't it possible for instance to just merge the backend
> > core (and console/xenfb updates) in Ian's tree first for a start?
> 
> http://kraxel.fedorapeople.org/patches/qemu-xen/

Ok, thanks, that's already better.

I'd still say that "delete a file then add another one" is far from
convenient both from a point of view of proof-reading the patches and
repository history, but I guess we can manage that with a renaming step
first.

Any reason for the renames, though? (they tend to bother developpers who
have to change their habits, so we can not do that without a reason)
- Why dashes instead of underscores?
- In Xen, we call xen-machine.c xen_machine_pv.c because there is also
a xen_machine_fv.c.  Can't the xen_machine_pv.c name be fine for qemu
upstream too?  Or xen can keep its own xen_machine_{pv,fv}.c and your
xen-machine.c goes upstream, I don't think that would be a problem.
- I guess a xenfb -> xen[-_]{fb,framebuffer} rename is indeed more
coherent, Markus, do you prefer just "fb" or the longer "framebuffer"?


Misc stuff:
- I guess the sys-queue.h file should be merged and used in qemu first?
- xen_domid is re-declared in xen-backend.h
- The BUFSIZE macro in xen-backend.h is a bit unfortunate, but still
better than what we have currently.  Maybe it should be renamed with a
- XEN_ prefix to avoid any clash?
- container_of would probably be useful in other parts of qemu, I guess
it could be merged in qemu first?
- nice work on moving stuff from console and fb to the backend, console
is easier to read now :)

> I also largely left vl.c as-is, so xend shouldn't need any changes.  The
> -domid switch sets an additional (redundant) variable, to keep the
> amount of changes as small as possible for now.  Also -name and
> -domain-name are aliased, both set qemu_name and domain_name.

That's a good thing :)

> The framebuffer driver probably has some performance regressions.
> Fixing those depends on the display patches being pushed upstream.

It would have been easier to review if you had provided a delta patch,
not a delete/add patch </grin>.

That's actually the kind of things I was afraid of and that would have
been harder to spot when just pulling your work from qemu upstream.

In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
can just use them.  We'll push them to qemu.  We'll manage the _shared
stuff (and push it eventually).

I'll let Markus comment on the rest (again, thanks for moving the xenbus
stuff to the backend part).  There is one change that is not backend
changes or just moving code around: you are queuing rectangle updates,
why?  (I'm not arguing, just wondering the kind of optimization that can
be).

> > Then you can push your code to qemu,
> > I guess that could be fine, as you said xen will not need to use e.g.
> > the block and net backends.
> 
> blk and net backends are not there (yet).  But they should be a nop for
> xen anyway

Indeed, I'd say don't bother to port those.

> The block backend should be a good replacement for blktap though and
> maybe can save you the effort of porting the blktap kernel driver to
> the pv_ops kernel.

Well, for better performance an in-kernel blktap would still be useful.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

> Ok, thanks, that's already better.
> 
> I'd still say that "delete a file then add another one" is far from
> convenient both from a point of view of proof-reading the patches and
> repository history, but I guess we can manage that with a renaming step
> first.

Fine with me, I can't send patches with renames though ...

> Any reason for the renames, though? (they tend to bother developpers who
> have to change their habits, so we can not do that without a reason)

Get consistent naming (all xen stuff in hw/ is prefixed with xen-).

> - Why dashes instead of underscores?

(1) It looks more pleasent to my eyes.
(2) It is easier to type (no shift needed) on both us and german
    keyboards.
(3) The files in the qemu source tree don't have a consistent style
    in respect to '-' vs. '_', so I had no reason to not use my
    personal preference ;)

> - In Xen, we call xen-machine.c xen_machine_pv.c because there is also
> a xen_machine_fv.c.  Can't the xen_machine_pv.c name be fine for qemu
> upstream too?

Dunno whenever there ever will be a xenfv machine type upstream as most
likely the normal 'pc' machine type will get a fancy interface to switch
between emulation, kqemu and kvm.  And I think the best option for xen
hvm would be to hook in there too.  That is a long-term thing though,
xen_machine_fv.c will probably stay for quite a while at in the xen
tree, so maybe 'pv' in the name helps avoiding confusion.  I'd prefer to
stay with the xen-machine.c though.

> Or xen can keep its own xen_machine_{pv,fv}.c and your
> xen-machine.c goes upstream, I don't think that would be a problem.

We have two files defining xenpv_machine then.  I don't think it is a
good idea.

> Misc stuff:
> - I guess the sys-queue.h file should be merged and used in qemu first?

Rebase.  Then it will be there, and you can skip the patch ;)

> - xen_domid is re-declared in xen-backend.h

Oops.  Fixed.

> - The BUFSIZE macro in xen-backend.h is a bit unfortunate, but still
> better than what we have currently.  Maybe it should be renamed with a
> - XEN_ prefix to avoid any clash?

xen-backend.h is supposed to be included by xen-related code only (it
barfs when /usr/include/xen isn't there), so it shouldn't clash in
theory.  Well, I can prefix it nevertheless, better safe than sorry.

> - container_of would probably be useful in other parts of qemu, I guess
> it could be merged in qemu first?

Has qemu a nice place for that kind of helper macros?

> In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
> can just use them.  We'll push them to qemu.  We'll manage the _shared
> stuff (and push it eventually).

I'd prefer to do it the other way around (push depending changes
upstream, then adapt xen-framebuffer.c).  I want xen-framebuffer.c look
the same in xen and upstream.

> I'll let Markus comment on the rest (again, thanks for moving the xenbus
> stuff to the backend part).  There is one change that is not backend
> changes or just moving code around: you are queuing rectangle updates,
> why?  (I'm not arguing, just wondering the kind of optimization that can
> be).

Thats actually a bugfix.  Doing screen updates outside the ->update
callback isn't safe.

And as we must queuing up update rectangles anyway to fix that it also
tries to optimize stuff a bit and avoid unneeded bitblits.  Right now
that catches only frequent fullscreen updates (such as a scrolling
textconsole) and doesn't copy stuff multiple times in case the update
events from the guest are more frequent than update callback calls from
qemu.

Could be improved to analyze the rectangles in more detail (for example:
figure a update is a superset of another one, so one can be dropped).

>> The block backend should be a good replacement for blktap though and
>> maybe can save you the effort of porting the blktap kernel driver to
>> the pv_ops kernel.
> 
> Well, for better performance an in-kernel blktap would still be useful.

Don't confuse blkback and blktap.  I don't want to replace the in-kernel
blkback driver.  blktap isn't a in-kernel driver though.  It is just an
interface between the hypervisor and a userspace application,
specialized for block devices.  My backend driver does pretty much the
same, but uses the generic gntdev interface instead of blktap.  I can't
see any reason why it shouldn't match blktap performance-wise.  I have
no benchmarks numbers though.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> > Any reason for the renames, though? (they tend to bother developpers who
> > have to change their habits, so we can not do that without a reason)
> 
> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).

Err, no, in xen they are all prefixed with xen_ (except xenfb).

> > - Why dashes instead of underscores?
> 
> (1) It looks more pleasent to my eyes.
> (2) It is easier to type (no shift needed) on both us and german
>     keyboards.

Not on fr keyboard :p

> (3) The files in the qemu source tree don't have a consistent style
>     in respect to '-' vs. '_',

There are far more _ than - in qemu. - seems to be only used for
things that just share a very generic idea (i.e. usb- and scsi-), while
_ seems to be used for things that are more closely related, like arm_*,
mips_*, ppc_*, ... xen_* would make sense to my mind.

> so I had no reason to not use my personal preference ;)

Yes, there is a reason: as I said, that puts a little burden on
developpers that have already been working on it in Xen for some time.
That also asks Ian to do the move, that makes history digging more
tricky, etc.

> I'd prefer to stay with the xen-machine.c though.

Ok for your taste, but as I said the burden involved by renamings looks
more important to me.

> > Misc stuff:
> > - I guess the sys-queue.h file should be merged and used in qemu first?
> 
> Rebase.  Then it will be there, and you can skip the patch ;)

Ah ok, sorry, then fine.

> > - The BUFSIZE macro in xen-backend.h is a bit unfortunate, but still
> > better than what we have currently.  Maybe it should be renamed with a
> > - XEN_ prefix to avoid any clash?
> 
> xen-backend.h is supposed to be included by xen-related code only (it
> barfs when /usr/include/xen isn't there), so it shouldn't clash in
> theory.

In theory yes, but even people working on the xen part could want to
define a very local BUFSIZE, and then they could get a clash. Of course
they can easily fix their code, but as you say

> Well, I can prefix it nevertheless, better safe than sorry.

> > - container_of would probably be useful in other parts of qemu, I guess
> > it could be merged in qemu first?
> 
> Has qemu a nice place for that kind of helper macros?

I'll leave that one for more experienced qemu people.

> > In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
> > can just use them.  We'll push them to qemu.  We'll manage the _shared
> > stuff (and push it eventually).
> 
> I'd prefer to do it the other way around (push depending changes
> upstream, then adapt xen-framebuffer.c).  I want xen-framebuffer.c look
> the same in xen and upstream.

Sure, it's just that your code could be merged earlier with these
modifications.  But well we can handle that later it's not a problem.

> > I'll let Markus comment on the rest (again, thanks for moving the xenbus
> > stuff to the backend part).  There is one change that is not backend
> > changes or just moving code around: you are queuing rectangle updates,
> > why?  (I'm not arguing, just wondering the kind of optimization that can
> > be).
> 
> Thats actually a bugfix.  Doing screen updates outside the ->update
> callback isn't safe.

Oh.  Well, that's the kind of comments which you could have put
somewhere along your patches, for us to not frown on it while
reviewing...

For more performance, maybe it'd be better to only move the dpy_update()
part. It's better to do the xenfb_guest_copy() immediately since the
source data is probably already hot in the cache.

> >> The block backend should be a good replacement for blktap though and
> >> maybe can save you the effort of porting the blktap kernel driver to
> >> the pv_ops kernel.
> > 
> > Well, for better performance an in-kernel blktap would still be useful.
> 
> Don't confuse blkback and blktap.  I don't want to replace the in-kernel
> blkback driver.  blktap isn't a in-kernel driver though.

Well, actually _you_ confused me about this above :)

> My backend driver does pretty much the same, but uses the generic
> gntdev interface instead of blktap.  I can't see any reason why it
> shouldn't match blktap performance-wise.

That's possible indeed.

> I have no benchmarks numbers though.

That'd be good :)

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Blue Swirl]

On 8/7/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Samuel Thibault wrote:
>  > Samuel Thibault, le Wed 06 Aug 2008 15:25:26 +0100, a écrit :
>  >> Pushing the cleaning changes to Xen first can be done and would entail
>  >> much easier to tackle breakage, and the merge back from qemu would then
>  >> be trivial, why not doing so?
>  >
>  > You didn't answer that part.  Really, my only concern is about having
>  > things tested.  Isn't it possible for instance to just merge the backend
>  > core (and console/xenfb updates) in Ian's tree first for a start?
>
>
> http://kraxel.fedorapeople.org/patches/qemu-xen/
>
>  I didn't touch the build system, it is even more scary than the qemu one
>  alone, I just set CONFIG_XEN unconditionally.
>
>  I also largely left vl.c as-is, so xend shouldn't need any changes.  The
>  -domid switch sets an additional (redundant) variable, to keep the
>  amount of changes as small as possible for now.  Also -name and
>  -domain-name are aliased, both set qemu_name and domain_name.

0004-xenpv-groundwork.patch

You dropped nodisk_ok support from vl.c.

0005-xen-backend-core.patch

container_of could go to osdep.h.

0006-xen-console-backend.patch

Not your fault, but a lot of places (at least ps2.c and
slavio_serial.c) define some kind of ring buffers. Maybe these could
be consolidated some time.

0007-xen-framebuffer-backend.patch

After you optimized scancode2linux[], it looks like
atkbd_set2_keycode[] and atkbd_unxlate_table[] are not needed.

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Blue Swirl, le Thu 07 Aug 2008 18:06:07 +0300, a écrit :
> After you optimized scancode2linux[], it looks like
> atkbd_set2_keycode[] and atkbd_unxlate_table[] are not needed.

They should however probably be kept as comments just to know where
scancode2linux comes from.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Blue Swirl, le Thu 07 Aug 2008 18:06:07 +0300, a écrit :
> 0004-xenpv-groundwork.patch
> 
> You dropped nodisk_ok support from vl.c.

I guess that's ok for a merge in Ian's tree, which probably has it
already in another form.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Blue Swirl wrote:
>> http://kraxel.fedorapeople.org/patches/qemu-xen/

> 0004-xenpv-groundwork.patch
> 
> You dropped nodisk_ok support from vl.c.

Oh, there are *two* patchsets:
  http://kraxel.fedorapeople.org/patches/qemu-upstream/
  http://kraxel.fedorapeople.org/patches/qemu-xen/

The first also includes some not-yet submitted work-in-progress bits.
It is against upstream qemu svn.  The second is against xen's qemu fork,
so the xen-related changes can get a test-drive there.

Especially the vl.c changes are very different in the two patch sets
because xen's vl.c looks very different (lot of CONFIG_DM conditional
code) and because I don't want to break the command line interface for
now to not break xend's expectations.

That doesn't mean that the nodisk_ok got lost.  It is only in the first
patchset though.

> 0005-xen-backend-core.patch
> 
> container_of could go to osdep.h.

Will do.

> 0006-xen-console-backend.patch
> 
> Not your fault, but a lot of places (at least ps2.c and
> slavio_serial.c) define some kind of ring buffers. Maybe these could
> be consolidated some time.

Will have a look if I find some time.

> 0007-xen-framebuffer-backend.patch
> 
> After you optimized scancode2linux[], it looks like
> atkbd_set2_keycode[] and atkbd_unxlate_table[] are not needed.

Indeed.  Think I just ifdef them out and leave them in there as
documentation.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> > Or xen can keep its own xen_machine_{pv,fv}.c and your
> > xen-machine.c goes upstream, I don't think that would be a problem.
> 
> We have two files defining xenpv_machine then.  I don't think it is a
> good idea.

I guess Ian would just drop the xen_machine.c file in his tree.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Samuel Thibault wrote:
> Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
>>> Any reason for the renames, though? (they tend to bother developpers who
>>> have to change their habits, so we can not do that without a reason)
>> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).
> 
> Err, no, in xen they are all prefixed with xen_ (except xenfb).

Uhm, No.

~/xen/qemu-dm# grep ^OBJS xen-hooks.mak
OBJS += piix4acpi.o
OBJS += xenstore.o
OBJS += xen_platform.o
OBJS += xen_machine_fv.o
OBJS += xen_machine_fv.o
OBJS += xen_blktap.o
OBJS += exec-dm.o
OBJS += pci_emulation.o
OBJS += tpm_tis.o
OBJS+= pass-through.o pt-msi.o
OBJS := $(filter-out $(BAD_OBJS), $(OBJS))

That is neither consistent wrt using _ everythere nor all files are
prefixed consistently.  At least all prefixed ones use underscores.

>> (3) The files in the qemu source tree don't have a consistent style
>>     in respect to '-' vs. '_',
> 
> There are far more _ than - in qemu. - seems to be only used for
> things that just share a very generic idea (i.e. usb- and scsi-), while
> _ seems to be used for things that are more closely related, like arm_*,
> mips_*, ppc_*, ... xen_* would make sense to my mind.

To me it looks pretty random, probably depending on the person creating
that file.  And when you count them, then there is no clear winner:

~/projects/qemu# find -name "*.[ch]" -print | grep "-" | wc -l
293
~/projects/qemu# find -name "*.[ch]" -print | grep "_" | wc -l
231

>> so I had no reason to not use my personal preference ;)
> 
> Yes, there is a reason: as I said, that puts a little burden on
> developpers that have already been working on it in Xen for some time.
> That also asks Ian to do the move, that makes history digging more
> tricky, etc.

git handles renames just fine.

> For more performance, maybe it'd be better to only move the dpy_update()
> part. It's better to do the xenfb_guest_copy() immediately since the
> source data is probably already hot in the cache.

No.  The copy is unsafe.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> > In your xen patch, since idle and GUI_REFRESH_INTERVAL are there, you
> > can just use them.  We'll push them to qemu.
> 
> I'd prefer to do it the other way around (push depending changes
> upstream, then adapt xen-framebuffer.c).  I want xen-framebuffer.c look
> the same in xen and upstream.

Mmm, looking at that again, most of my idleness patch is actually
already upstream.  What wasn't pushed is what is specific to xenfb:
transmitting the idleness to the backend.  For this xenfb needs to get
idleness information.  Below is the patch that does that, you can just
fold it in your qemu-upstream/0008-xen-add-framebuffer-backend-driver.patch
About GUI_REFRESH_INTERVAL, you can just move it to a header.

Samuel

Index: console.h
===================================================================
--- console.h	(révision 4992)
+++ console.h	(copie de travail)
@@ -80,6 +80,7 @@
     void *opaque;
     struct QEMUTimer *gui_timer;
     uint64_t gui_timer_interval;
+    int idle;
 
     void (*dpy_update)(struct DisplayState *s, int x, int y, int w, int h);
     void (*dpy_resize)(struct DisplayState *s, int w, int h);
Index: vl.c
===================================================================
--- vl.c	(révision 4992)
+++ vl.c	(copie de travail)
@@ -5930,6 +5930,8 @@
     ds->dpy_update = dumb_update;
     ds->dpy_resize = dumb_resize;
     ds->dpy_refresh = dumb_refresh;
+    ds->gui_timer_interval = 500;
+    ds->idle = 1;
 }
 
 /***********************************************************/
Index: sdl.c
===================================================================
--- sdl.c	(révision 4992)
+++ sdl.c	(copie de travail)
@@ -526,9 +526,11 @@
                 if (ev->active.gain) {
                     /* Back to default interval */
                     ds->gui_timer_interval = 0;
+                    ds->idle = 0;
                 } else {
                     /* Sleeping interval */
                     ds->gui_timer_interval = 500;
+                    ds->idle = 1;
                 }
             }
             break;
Index: vnc.c
===================================================================
--- vnc.c	(révision 4992)
+++ vnc.c	(copie de travail)
@@ -660,6 +660,7 @@  client_io_error
 	qemu_set_fd_handler2(vs->csock, NULL, NULL, NULL, NULL);
 	closesocket(vs->csock);
 	vs->csock = -1;
+	vs->ds->idle = 1;
 	buffer_reset(&vs->input);
 	buffer_reset(&vs->output);
 	vs->need_update = 0;
@@ -1920,6 +1921,7 @@
 static void vnc_connect(VncState *vs)
 {
     VNC_DEBUG("New client on socket %d\n", vs->csock);
+    vs->ds->idle = 0;
     socket_set_nonblock(vs->csock);
     qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
     vnc_write(vs, "RFB 003.008\n", 12);
@@ -1959,6 +1961,7 @@
 	exit(1);
 
     ds->opaque = vs;
+    ds->idle = 1;
     vnc_state = vs;
     vs->display = NULL;
     vs->password = NULL;

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Gerd Hoffmann, le Thu 07 Aug 2008 18:17:39 +0200, a écrit :
> Samuel Thibault wrote:
> > Gerd Hoffmann, le Thu 07 Aug 2008 14:13:22 +0200, a écrit :
> >>> Any reason for the renames, though? (they tend to bother developpers who
> >>> have to change their habits, so we can not do that without a reason)
> >> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).
> > 
> > Err, no, in xen they are all prefixed with xen_ (except xenfb).
> 
> Uhm, No.

Right, there is xenstore as well.

> ~/xen/qemu-dm# grep ^OBJS xen-hooks.mak
> OBJS += piix4acpi.o
> [snip xen*]
> OBJS += exec-dm.o
> OBJS += pci_emulation.o
> OBJS += tpm_tis.o
> OBJS+= pass-through.o pt-msi.o
> OBJS := $(filter-out $(BAD_OBJS), $(OBJS))

These aren't really xen-specific, that's why they don't have a xen or
xen_ prefix.

> That is neither consistent wrt using _ everythere nor all files are
> prefixed consistently.  At least all prefixed ones use underscores.

And that's my point.  I don't see why we should take the burden of
renaming them with dashes.

> >> (3) The files in the qemu source tree don't have a consistent style
> >>     in respect to '-' vs. '_',
> > 
> > There are far more _ than - in qemu.

Just to comment on that. I actually meant in hw/ . There are a lot of -
in the root, because there are block-*, qemu-*, cpu-*, config-*, etc.

> - seems to be only used for
> > things that just share a very generic idea (i.e. usb- and scsi-), while
> > _ seems to be used for things that are more closely related, like arm_*,
> > mips_*, ppc_*, ... xen_* would make sense to my mind.
> 
> To me it looks pretty random,

I doesn't look so much random to me. There are oddities, but the rule
above seems mostly respected.

> And when you count them, then there is no clear winner:
> 
> ~/projects/qemu# find -name "*.[ch]" -print | grep "-" | wc -l
> 293
> ~/projects/qemu# find -name "*.[ch]" -print | grep "_" | wc -l
> 231

Sure, they have different purpose.  As I said, _ for closely related
(like must be compiled together), - for not closely related (i.e.
independant matter that just have some generic link, like the block
interface, scsi or usb bus).

> >> so I had no reason to not use my personal preference ;)
> > 
> > Yes, there is a reason: as I said, that puts a little burden on
> > developpers that have already been working on it in Xen for some time.
> > That also asks Ian to do the move, that makes history digging more
> > tricky, etc.
> 
> git handles renames just fine.

Yes, sure, that's what I meant before ("having a renaming step first").
But that's still work to actually do it, change the Makefiles, and then
when you want to git annotate an old version it becomes tricky: you have
to remember the old name.  So renaming really needs a reason.

> > For more performance, maybe it'd be better to only move the dpy_update()
> > part. It's better to do the xenfb_guest_copy() immediately since the
> > source data is probably already hot in the cache.
> 
> No.  The copy is unsafe.

Ah, because we're writing to ds->data which is handled by the display
backend, right.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Samuel Thibault, le Thu 07 Aug 2008 17:48:43 +0100, a écrit :
> > OBJS += exec-dm.o
> 
> These aren't really xen-specific, that's why they don't have a xen or
> xen_ prefix.

Except this one of course since it's the way it's supposed to be called.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Stefano Stabellini]

Gerd Hoffmann wrote:

> It would also be great if you can clean up the header mess.  qemu-dm
> doesn't build without xen-unstable.hg.  And just a checked out tree
> isn't good enougth, it must be compiled because qemu-dm depends on some
> generated header files.  Aiee.  IMHO qemu-dm should only need
> /usr/include/xen.  If headers are missing there, they should be installed.
> 

It is not difficult to make Ian's qemu tree compile against
/usr/include/xen.
Expect a patch to fix this on Monday.

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Samuel Thibault]

Blue Swirl, le Thu 07 Aug 2008 18:06:07 +0300, a écrit :
> 0006-xen-console-backend.patch
> 
> Not your fault, but a lot of places (at least ps2.c and
> slavio_serial.c) define some kind of ring buffers. Maybe these could
> be consolidated some time.

There is also one in console.c
It'd be probably be good to have an implementation of a ring of
arbitrary structures.  We have a pending patch of usb-hid to fix double
clicks by queueing click events.

Samuel

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

On 8/7/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Blue Swirl wrote:
>  >> http://kraxel.fedorapeople.org/patches/qemu-xen/
>
>
> > 0004-xenpv-groundwork.patch
>  >
>  > You dropped nodisk_ok support from vl.c.
>
>
> Oh, there are *two* patchsets:
>   http://kraxel.fedorapeople.org/patches/qemu-upstream/
>
>   http://kraxel.fedorapeople.org/patches/qemu-xen/
>
>
> The first also includes some not-yet submitted work-in-progress bits.
>  It is against upstream qemu svn.  The second is against xen's qemu fork,
>  so the xen-related changes can get a test-drive there.
>
>  Especially the vl.c changes are very different in the two patch sets
>  because xen's vl.c looks very different (lot of CONFIG_DM conditional
>  code) and because I don't want to break the command line interface for
>  now to not break xend's expectations.
>
>  That doesn't mean that the nodisk_ok got lost.  It is only in the first
>  patchset though.

I extracted a small patch that only introduces the nodisk_ok part.
Anybody unhappy if I commit it?

[-- Attachment #2: nodisk.diff --]
[-- Type: plain/text, Size: 3351 bytes --]

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

  Hi,

> Mmm, looking at that again, most of my idleness patch is actually
> already upstream.  What wasn't pushed is what is specific to xenfb:
> transmitting the idleness to the backend.  For this xenfb needs to get
> idleness information.  Below is the patch that does that, you can just
> fold it in your qemu-upstream/0008-xen-add-framebuffer-backend-driver.patch
> About GUI_REFRESH_INTERVAL, you can just move it to a header.

Thanks, I'll stick it into my patch queue.

That leaves the DisplayState->shared_buf bits which should be upstreamed
too ...

cheers,
  Gerd

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Ian Jackson]

Gerd Hoffmann writes ("Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
> IMHO the best way to address this issue is to rebase frequently.  The
> xen-unstable testing will also covers qemu then.  It also keeps the
> delta smaller and makes it easier to test qemu patches in qemu-dm.

I'm not going to be rebasing, but I will merge instead.

> I'm trying to fiddle my bits into qemu-dm right now.  One problem is
> that I get build failures simply because of the fact that the qemu-dm
> base is old compared to upstream qemu.

Yes, that's currently the case because qemu-xen-unstable is frozen for
the forthcoming Xen release.

> It would also be great if you can clean up the header mess.  qemu-dm
> doesn't build without xen-unstable.hg.

This is true and is likely to continue to be the case indefinitely.

>  And just a checked out tree
> isn't good enougth, it must be compiled because qemu-dm depends on some
> generated header files.  Aiee.  IMHO qemu-dm should only need
> /usr/include/xen.  If headers are missing there, they should be installed.

qemu-dm also needs libxc, libthis, libthat from xen-unstable.

Ian.

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Stefano Stabellini]

Gerd Hoffmann wrote:

>   Hi,
> 
>> Mmm, looking at that again, most of my idleness patch is actually
>> already upstream.  What wasn't pushed is what is specific to xenfb:
>> transmitting the idleness to the backend.  For this xenfb needs to get
>> idleness information.  Below is the patch that does that, you can just
>> fold it in your qemu-upstream/0008-xen-add-framebuffer-backend-driver.patch
>> About GUI_REFRESH_INTERVAL, you can just move it to a header.
> 
> Thanks, I'll stick it into my patch queue.
> 
> That leaves the DisplayState->shared_buf bits which should be upstreamed
> too ...
> 

It is the next item on my TODO :)

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Ian Jackson]

Gerd Hoffmann writes ("Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu"):
> [Samuel asks:]
> > - Why dashes instead of underscores?
> 
> (1) It looks more pleasent to my eyes.
> (2) It is easier to type (no shift needed) on both us and german
>     keyboards.
> (3) The files in the qemu source tree don't have a consistent style
>     in respect to '-' vs. '_', so I had no reason to not use my
>     personal preference ;)

I basically agree with this.  However:

> > Any reason for the renames, though? (they tend to bother developpers who
> > have to change their habits, so we can not do that without a reason)
> 
> Get consistent naming (all xen stuff in hw/ is prefixed with xen-).

I absolutely and vehemently disagree that files should be renamed (or
indeed code style changed) to make things prettier or more consistent.

There are currently at least three substantial branches and numerous
smaller branches of this xen code in at least two different revision
control systems - and there are probably many more.

The very last thing we want is to make porting patches between the
various branches difficult.

Ian.

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Ian Jackson wrote:
>>  And just a checked out tree
>> isn't good enougth, it must be compiled because qemu-dm depends on some
>> generated header files.  Aiee.  IMHO qemu-dm should only need
>> /usr/include/xen.  If headers are missing there, they should be installed.
> 
> qemu-dm also needs libxc, libthis, libthat from xen-unstable.

That kind of stuff is usually packed up as xen-devel in linux
distributions.  I want being able to just install xen-devel on my
machine, then compile qemu with xen support.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Gerd Hoffmann]

Blue Swirl wrote:
> I extracted a small patch that only introduces the nodisk_ok part.
> Anybody unhappy if I commit it?

How about converting all struct fields to C99 initializers in
hw/sun4*.c?  Mixing the two styles looks quite odd.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

Re: [Xen-devel] Re: [Qemu-devel] [PATCH 0/7] merge some xen bits into qemu

[Blue Swirl]

On 8/11/08, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Blue Swirl wrote:
>  > I extracted a small patch that only introduces the nodisk_ok part.
>  > Anybody unhappy if I commit it?
>
>
> How about converting all struct fields to C99 initializers in
>  hw/sun4*.c?  Mixing the two styles looks quite odd.

Good idea, I'll do the switch.