* Re: Flow Control and Port Mirroring Revisited
From: Michael S. Tsirkin @ 2011-01-14 6:54 UTC (permalink / raw)
To: Simon Horman
Cc: Jesse Gross, Eric Dumazet, Rusty Russell, virtualization, dev,
virtualization, netdev, kvm
In-Reply-To: <20110114063528.GB10957@verge.net.au>
On Fri, Jan 14, 2011 at 03:35:28PM +0900, Simon Horman wrote:
> On Fri, Jan 14, 2011 at 06:58:18AM +0200, Michael S. Tsirkin wrote:
> > On Fri, Jan 14, 2011 at 08:41:36AM +0900, Simon Horman wrote:
> > > On Thu, Jan 13, 2011 at 10:45:38AM -0500, Jesse Gross wrote:
> > > > On Thu, Jan 13, 2011 at 1:47 AM, Simon Horman <horms@verge.net.au> wrote:
> > > > > On Mon, Jan 10, 2011 at 06:31:55PM +0900, Simon Horman wrote:
> > > > >> On Fri, Jan 07, 2011 at 10:23:58AM +0900, Simon Horman wrote:
> > > > >> > On Thu, Jan 06, 2011 at 05:38:01PM -0500, Jesse Gross wrote:
> > > > >> >
> > > > >> > [ snip ]
> > > > >> > >
> > > > >> > > I know that everyone likes a nice netperf result but I agree with
> > > > >> > > Michael that this probably isn't the right question to be asking. I
> > > > >> > > don't think that socket buffers are a real solution to the flow
> > > > >> > > control problem: they happen to provide that functionality but it's
> > > > >> > > more of a side effect than anything. It's just that the amount of
> > > > >> > > memory consumed by packets in the queue(s) doesn't really have any
> > > > >> > > implicit meaning for flow control (think multiple physical adapters,
> > > > >> > > all with the same speed instead of a virtual device and a physical
> > > > >> > > device with wildly different speeds). The analog in the physical
> > > > >> > > world that you're looking for would be Ethernet flow control.
> > > > >> > > Obviously, if the question is limiting CPU or memory consumption then
> > > > >> > > that's a different story.
> > > > >> >
> > > > >> > Point taken. I will see if I can control CPU (and thus memory) consumption
> > > > >> > using cgroups and/or tc.
> > > > >>
> > > > >> I have found that I can successfully control the throughput using
> > > > >> the following techniques
> > > > >>
> > > > >> 1) Place a tc egress filter on dummy0
> > > > >>
> > > > >> 2) Use ovs-ofctl to add a flow that sends skbs to dummy0 and then eth1,
> > > > >> this is effectively the same as one of my hacks to the datapath
> > > > >> that I mentioned in an earlier mail. The result is that eth1
> > > > >> "paces" the connection.
> >
> > This is actually a bug. This means that one slow connection will affect
> > fast ones. I intend to change the default for qemu to sndbuf=0 : this
> > will fix it but break your "pacing". So pls do not count on this
> > behaviour.
>
> Do you have a patch I could test?
You can (and users already can) just run qemu with sndbuf=0. But if you
like, below.
> > > > > Further to this, I wonder if there is any interest in providing
> > > > > a method to switch the action order - using ovs-ofctl is a hack imho -
> > > > > and/or switching the default action order for mirroring.
> > > >
> > > > I'm not sure that there is a way to do this that is correct in the
> > > > generic case. It's possible that the destination could be a VM while
> > > > packets are being mirrored to a physical device or we could be
> > > > multicasting or some other arbitrarily complex scenario. Just think
> > > > of what a physical switch would do if it has ports with two different
> > > > speeds.
> > >
> > > Yes, I have considered that case. And I agree that perhaps there
> > > is no sensible default. But perhaps we could make it configurable somehow?
> >
> > The fix is at the application level. Run netperf with -b and -w flags to
> > limit the speed to a sensible value.
>
> Perhaps I should have stated my goals more clearly.
> I'm interested in situations where I don't control the application.
Well an application that streams UDP without any throttling
at the application level will break on a physical network, right?
So I am not sure why should one try to make it work on the virtual one.
But let's assume that you do want to throttle the guest
for reasons such as QOS. The proper approach seems
to be to throttle the sender, not have a dummy throttled
receiver "pacing" it. Place the qemu process in the
correct net_cls cgroup, set the class id and apply a rate limit?
---
diff --git a/net/tap-linux.c b/net/tap-linux.c
index f7aa904..0dbcdd4 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -87,7 +87,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required
* Ethernet NICs generally have txqueuelen=1000, so 1Mb is
* a good default, given a 1500 byte MTU.
*/
-#define TAP_DEFAULT_SNDBUF 1024*1024
+#define TAP_DEFAULT_SNDBUF 0
int tap_set_sndbuf(int fd, QemuOpts *opts)
{
^ permalink raw reply related
* Re: Flow Control and Port Mirroring Revisited
From: Simon Horman @ 2011-01-14 6:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jesse Gross, Eric Dumazet, Rusty Russell, virtualization, dev,
virtualization, netdev, kvm
In-Reply-To: <20110114045818.GA29738@redhat.com>
On Fri, Jan 14, 2011 at 06:58:18AM +0200, Michael S. Tsirkin wrote:
> On Fri, Jan 14, 2011 at 08:41:36AM +0900, Simon Horman wrote:
> > On Thu, Jan 13, 2011 at 10:45:38AM -0500, Jesse Gross wrote:
> > > On Thu, Jan 13, 2011 at 1:47 AM, Simon Horman <horms@verge.net.au> wrote:
> > > > On Mon, Jan 10, 2011 at 06:31:55PM +0900, Simon Horman wrote:
> > > >> On Fri, Jan 07, 2011 at 10:23:58AM +0900, Simon Horman wrote:
> > > >> > On Thu, Jan 06, 2011 at 05:38:01PM -0500, Jesse Gross wrote:
> > > >> >
> > > >> > [ snip ]
> > > >> > >
> > > >> > > I know that everyone likes a nice netperf result but I agree with
> > > >> > > Michael that this probably isn't the right question to be asking. I
> > > >> > > don't think that socket buffers are a real solution to the flow
> > > >> > > control problem: they happen to provide that functionality but it's
> > > >> > > more of a side effect than anything. It's just that the amount of
> > > >> > > memory consumed by packets in the queue(s) doesn't really have any
> > > >> > > implicit meaning for flow control (think multiple physical adapters,
> > > >> > > all with the same speed instead of a virtual device and a physical
> > > >> > > device with wildly different speeds). The analog in the physical
> > > >> > > world that you're looking for would be Ethernet flow control.
> > > >> > > Obviously, if the question is limiting CPU or memory consumption then
> > > >> > > that's a different story.
> > > >> >
> > > >> > Point taken. I will see if I can control CPU (and thus memory) consumption
> > > >> > using cgroups and/or tc.
> > > >>
> > > >> I have found that I can successfully control the throughput using
> > > >> the following techniques
> > > >>
> > > >> 1) Place a tc egress filter on dummy0
> > > >>
> > > >> 2) Use ovs-ofctl to add a flow that sends skbs to dummy0 and then eth1,
> > > >> this is effectively the same as one of my hacks to the datapath
> > > >> that I mentioned in an earlier mail. The result is that eth1
> > > >> "paces" the connection.
>
> This is actually a bug. This means that one slow connection will affect
> fast ones. I intend to change the default for qemu to sndbuf=0 : this
> will fix it but break your "pacing". So pls do not count on this
> behaviour.
Do you have a patch I could test?
> > > > Further to this, I wonder if there is any interest in providing
> > > > a method to switch the action order - using ovs-ofctl is a hack imho -
> > > > and/or switching the default action order for mirroring.
> > >
> > > I'm not sure that there is a way to do this that is correct in the
> > > generic case. It's possible that the destination could be a VM while
> > > packets are being mirrored to a physical device or we could be
> > > multicasting or some other arbitrarily complex scenario. Just think
> > > of what a physical switch would do if it has ports with two different
> > > speeds.
> >
> > Yes, I have considered that case. And I agree that perhaps there
> > is no sensible default. But perhaps we could make it configurable somehow?
>
> The fix is at the application level. Run netperf with -b and -w flags to
> limit the speed to a sensible value.
Perhaps I should have stated my goals more clearly.
I'm interested in situations where I don't control the application.
^ permalink raw reply
* Re: Flow Control and Port Mirroring Revisited
From: Michael S. Tsirkin @ 2011-01-14 4:58 UTC (permalink / raw)
To: Simon Horman
Cc: Jesse Gross, Eric Dumazet, Rusty Russell, virtualization, dev,
virtualization, netdev, kvm
In-Reply-To: <20110113234135.GC8426@verge.net.au>
On Fri, Jan 14, 2011 at 08:41:36AM +0900, Simon Horman wrote:
> On Thu, Jan 13, 2011 at 10:45:38AM -0500, Jesse Gross wrote:
> > On Thu, Jan 13, 2011 at 1:47 AM, Simon Horman <horms@verge.net.au> wrote:
> > > On Mon, Jan 10, 2011 at 06:31:55PM +0900, Simon Horman wrote:
> > >> On Fri, Jan 07, 2011 at 10:23:58AM +0900, Simon Horman wrote:
> > >> > On Thu, Jan 06, 2011 at 05:38:01PM -0500, Jesse Gross wrote:
> > >> >
> > >> > [ snip ]
> > >> > >
> > >> > > I know that everyone likes a nice netperf result but I agree with
> > >> > > Michael that this probably isn't the right question to be asking. I
> > >> > > don't think that socket buffers are a real solution to the flow
> > >> > > control problem: they happen to provide that functionality but it's
> > >> > > more of a side effect than anything. It's just that the amount of
> > >> > > memory consumed by packets in the queue(s) doesn't really have any
> > >> > > implicit meaning for flow control (think multiple physical adapters,
> > >> > > all with the same speed instead of a virtual device and a physical
> > >> > > device with wildly different speeds). The analog in the physical
> > >> > > world that you're looking for would be Ethernet flow control.
> > >> > > Obviously, if the question is limiting CPU or memory consumption then
> > >> > > that's a different story.
> > >> >
> > >> > Point taken. I will see if I can control CPU (and thus memory) consumption
> > >> > using cgroups and/or tc.
> > >>
> > >> I have found that I can successfully control the throughput using
> > >> the following techniques
> > >>
> > >> 1) Place a tc egress filter on dummy0
> > >>
> > >> 2) Use ovs-ofctl to add a flow that sends skbs to dummy0 and then eth1,
> > >> this is effectively the same as one of my hacks to the datapath
> > >> that I mentioned in an earlier mail. The result is that eth1
> > >> "paces" the connection.
This is actually a bug. This means that one slow connection will
affect fast ones. I intend to change the default for qemu to sndbuf=0 :
this will fix it but break your "pacing". So pls do not count on this behaviour.
> > > Further to this, I wonder if there is any interest in providing
> > > a method to switch the action order - using ovs-ofctl is a hack imho -
> > > and/or switching the default action order for mirroring.
> >
> > I'm not sure that there is a way to do this that is correct in the
> > generic case. It's possible that the destination could be a VM while
> > packets are being mirrored to a physical device or we could be
> > multicasting or some other arbitrarily complex scenario. Just think
> > of what a physical switch would do if it has ports with two different
> > speeds.
>
> Yes, I have considered that case. And I agree that perhaps there
> is no sensible default. But perhaps we could make it configurable somehow?
The fix is at the application level. Run netperf with -b and -w flags to
limit the speed to a sensible value.
--
MST
^ permalink raw reply
* Re: Flow Control and Port Mirroring Revisited
From: Simon Horman @ 2011-01-13 23:41 UTC (permalink / raw)
To: Jesse Gross
Cc: Eric Dumazet, Rusty Russell, virtualization, dev, virtualization,
netdev, kvm, Michael S. Tsirkin
In-Reply-To: <AANLkTimO=5HmTJO1kmHGAWa-HTac+3d0TbrmJX5W4hVu@mail.gmail.com>
On Thu, Jan 13, 2011 at 10:45:38AM -0500, Jesse Gross wrote:
> On Thu, Jan 13, 2011 at 1:47 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Mon, Jan 10, 2011 at 06:31:55PM +0900, Simon Horman wrote:
> >> On Fri, Jan 07, 2011 at 10:23:58AM +0900, Simon Horman wrote:
> >> > On Thu, Jan 06, 2011 at 05:38:01PM -0500, Jesse Gross wrote:
> >> >
> >> > [ snip ]
> >> > >
> >> > > I know that everyone likes a nice netperf result but I agree with
> >> > > Michael that this probably isn't the right question to be asking. I
> >> > > don't think that socket buffers are a real solution to the flow
> >> > > control problem: they happen to provide that functionality but it's
> >> > > more of a side effect than anything. It's just that the amount of
> >> > > memory consumed by packets in the queue(s) doesn't really have any
> >> > > implicit meaning for flow control (think multiple physical adapters,
> >> > > all with the same speed instead of a virtual device and a physical
> >> > > device with wildly different speeds). The analog in the physical
> >> > > world that you're looking for would be Ethernet flow control.
> >> > > Obviously, if the question is limiting CPU or memory consumption then
> >> > > that's a different story.
> >> >
> >> > Point taken. I will see if I can control CPU (and thus memory) consumption
> >> > using cgroups and/or tc.
> >>
> >> I have found that I can successfully control the throughput using
> >> the following techniques
> >>
> >> 1) Place a tc egress filter on dummy0
> >>
> >> 2) Use ovs-ofctl to add a flow that sends skbs to dummy0 and then eth1,
> >> this is effectively the same as one of my hacks to the datapath
> >> that I mentioned in an earlier mail. The result is that eth1
> >> "paces" the connection.
> >
> > Further to this, I wonder if there is any interest in providing
> > a method to switch the action order - using ovs-ofctl is a hack imho -
> > and/or switching the default action order for mirroring.
>
> I'm not sure that there is a way to do this that is correct in the
> generic case. It's possible that the destination could be a VM while
> packets are being mirrored to a physical device or we could be
> multicasting or some other arbitrarily complex scenario. Just think
> of what a physical switch would do if it has ports with two different
> speeds.
Yes, I have considered that case. And I agree that perhaps there
is no sensible default. But perhaps we could make it configurable somehow?
^ permalink raw reply
* Re: Flow Control and Port Mirroring Revisited
From: Jesse Gross @ 2011-01-13 15:45 UTC (permalink / raw)
To: Simon Horman
Cc: Eric Dumazet, Rusty Russell, virtualization, dev, virtualization,
netdev, kvm, Michael S. Tsirkin
In-Reply-To: <20110113064718.GA17905@verge.net.au>
On Thu, Jan 13, 2011 at 1:47 AM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Jan 10, 2011 at 06:31:55PM +0900, Simon Horman wrote:
>> On Fri, Jan 07, 2011 at 10:23:58AM +0900, Simon Horman wrote:
>> > On Thu, Jan 06, 2011 at 05:38:01PM -0500, Jesse Gross wrote:
>> >
>> > [ snip ]
>> > >
>> > > I know that everyone likes a nice netperf result but I agree with
>> > > Michael that this probably isn't the right question to be asking. I
>> > > don't think that socket buffers are a real solution to the flow
>> > > control problem: they happen to provide that functionality but it's
>> > > more of a side effect than anything. It's just that the amount of
>> > > memory consumed by packets in the queue(s) doesn't really have any
>> > > implicit meaning for flow control (think multiple physical adapters,
>> > > all with the same speed instead of a virtual device and a physical
>> > > device with wildly different speeds). The analog in the physical
>> > > world that you're looking for would be Ethernet flow control.
>> > > Obviously, if the question is limiting CPU or memory consumption then
>> > > that's a different story.
>> >
>> > Point taken. I will see if I can control CPU (and thus memory) consumption
>> > using cgroups and/or tc.
>>
>> I have found that I can successfully control the throughput using
>> the following techniques
>>
>> 1) Place a tc egress filter on dummy0
>>
>> 2) Use ovs-ofctl to add a flow that sends skbs to dummy0 and then eth1,
>> this is effectively the same as one of my hacks to the datapath
>> that I mentioned in an earlier mail. The result is that eth1
>> "paces" the connection.
>
> Further to this, I wonder if there is any interest in providing
> a method to switch the action order - using ovs-ofctl is a hack imho -
> and/or switching the default action order for mirroring.
I'm not sure that there is a way to do this that is correct in the
generic case. It's possible that the destination could be a VM while
packets are being mirrored to a physical device or we could be
multicasting or some other arbitrarily complex scenario. Just think
of what a physical switch would do if it has ports with two different
speeds.
^ permalink raw reply
* CFP: International Conference on Software Engineering and Knowledge Engineering (SEKE2011)
From: Ming Zhao @ 2011-01-13 11:53 UTC (permalink / raw)
To: virtualization
CALL FOR PAPERS
The Twenty-Third International Conference on Software Engineering and Knowledge Engineering (SEKE 2011)
will be held at Eden Roc Renaissance Miami Beach, USA
from July 7 to July 9, 2011.
The conference website is: www.ksi.edu/seke/seke11.html
The conference aims at bringing together experts in software engineering and knowledge engineering to discuss on relevant results in either software engineering or knowledge engineering or both. Special emphasis will be put on the transference of methods between both domains.
Submission of papers and demos are both welcome.
TOPICS
Agent architectures, ontologies, languages and protocols
Multi-agent systems
Agent-based learning and knowledge discovery
Interface agents
Agent-based auctions and marketplaces
Artificial life and societies
Secure mobile and multi-agent systems
Mobile agents
Mobile Commerce Technology and Application Systems
Mobile Systems
Autonomic computing
Adaptive Systems
Integrity, Security, and Fault Tolerance
Reliability
Enterprise Software, Middleware, and Tools
Process and Workflow Management
E-Commerce Solutions and Applications
Industry System Experience and Report
Service-centric software engineering
Service oriented requirements engineering
Service oriented architectures
Middleware for service based systems
Service discovery and composition
Quality of services
Service level agreements (drafting, negotiation, monitoring and management)
Runtime service management
Semantic web
Requirements Engineering
Agent-based software engineering
Artificial Intelligence Approaches to Software Engineering
Component-Based Software Engineering
Automated Software Specification
Automated Software Design and Synthesis
Computer-Supported Cooperative Work
Embedded and Ubiquitous Software Engineering
Measurement and Empirical Software Engineering
Reverse Engineering
Programming Languages and Software Engineering
Patterns and Frameworks
Reflection and Metadata Approaches
Program Understanding
Knowledge Acquisition
Knowledge-Based and Expert Systems
Knowledge Representation and Retrieval
Knowledge Engineering Tools and Techniques
Time and Knowledge Management Tools
Knowledge Visualization
Data visualization
Uncertainty Knowledge Management
Ontologies and Methodologies
Learning Software Organization
Tutoring, Documentation Systems
Human-Computer Interaction
Multimedia Applications, Frameworks, and Systems
Multimedia and Hypermedia Software Engineering
Smart Spaces
Pervasive Computing
Swarm intelligence
Soft Computing
Software Architecture
Software Assurance
Software Domain Modeling and Meta-Modeling
Software dependability
Software economics
Software Engineering Decision Support
Software Engineering Tools and Environments
Software Maintenance and Evolution
Software Process Modeling
Software product lines
Software Quality
Software Reuse
Software Safety
Software Security
Software Engineering Case Study and Experience Reports
Web and text mining
Web-Based Tools, Applications and Environment
Web-Based Knowledge Management
Web-Based Tools, Systems, and Environments
Web and Data Mining
CONFERENCE HOTEL
The SEKE 2011 Conference will be held at the hotel Eden Roc Renaissance Miami Beach. The hotel has made available for these limited dates (7/3 - 7/11/2011) to SEKE 2011 attendees a discount rate of $112 US dollars per room, per night, single/double occupancy, not including sales tax.
INFORMATION FOR AUTHORS
Papers must be written in English. An electronic version (Postscript, PDF, or MS Word format) of the full paper should be submitted using the following URL: http://conf.ksi.edu/seke2011/submit/SubmitPaper.php (available after Jan 1, 2011). Please use Internet Explorer as the browser. Manuscript must include a 200-word abstract and no more than 6 pages of IEEE Manuscript for Conference Proceedings (include figures and references but exclude copyright form).
CONTACT INFORMATION
SEKE 2011 Conference Secretariat
Knowledge Systems Institute
3420 Main Street
Skokie, IL 60076 USA
Tel: 847-679-3135
Fax: 847-679-3166
E-mail: seke11@ksi.edu
IMPORTANT DATES
Paper submission due: March 7, 2011
Notification of acceptance: April 20, 2011
Early registration deadline: May 10, 2011
Camera-ready copy: May 10, 2011
^ permalink raw reply
* Re: Flow Control and Port Mirroring Revisited
From: Simon Horman @ 2011-01-13 6:47 UTC (permalink / raw)
To: Jesse Gross
Cc: Eric Dumazet, Rusty Russell, virtualization, dev, virtualization,
netdev, kvm, Michael S. Tsirkin
In-Reply-To: <20110110093155.GB13420@verge.net.au>
On Mon, Jan 10, 2011 at 06:31:55PM +0900, Simon Horman wrote:
> On Fri, Jan 07, 2011 at 10:23:58AM +0900, Simon Horman wrote:
> > On Thu, Jan 06, 2011 at 05:38:01PM -0500, Jesse Gross wrote:
> >
> > [ snip ]
> > >
> > > I know that everyone likes a nice netperf result but I agree with
> > > Michael that this probably isn't the right question to be asking. I
> > > don't think that socket buffers are a real solution to the flow
> > > control problem: they happen to provide that functionality but it's
> > > more of a side effect than anything. It's just that the amount of
> > > memory consumed by packets in the queue(s) doesn't really have any
> > > implicit meaning for flow control (think multiple physical adapters,
> > > all with the same speed instead of a virtual device and a physical
> > > device with wildly different speeds). The analog in the physical
> > > world that you're looking for would be Ethernet flow control.
> > > Obviously, if the question is limiting CPU or memory consumption then
> > > that's a different story.
> >
> > Point taken. I will see if I can control CPU (and thus memory) consumption
> > using cgroups and/or tc.
>
> I have found that I can successfully control the throughput using
> the following techniques
>
> 1) Place a tc egress filter on dummy0
>
> 2) Use ovs-ofctl to add a flow that sends skbs to dummy0 and then eth1,
> this is effectively the same as one of my hacks to the datapath
> that I mentioned in an earlier mail. The result is that eth1
> "paces" the connection.
Further to this, I wonder if there is any interest in providing
a method to switch the action order - using ovs-ofctl is a hack imho -
and/or switching the default action order for mirroring.
> 3) 2) + place a tc egress filter on eth1
>
> Which mostly makes sense to me although I am a little confused about
> why 1) needs a filter on dummy0 (a filter on eth1 has no effect)
> but 3) needs a filter on eth1 (a filter on dummy0 has no effect,
> even if the skb is sent to dummy0 last.
>
> I also had some limited success using CPU cgroups, though obviously
> that targets CPU usage and thus the effect on throughput is fairly course.
> In short, its a useful technique but not one that bares further
> discussion here.
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [Xen-devel] [PATCH 01/14] x86/ticketlock: clean up types and accessors
From: Konrad Rzeszutek Wilk @ 2011-01-11 17:21 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Xen-devel, Mathieu Desnoyers, Nick Piggin, Peter Zijlstra,
Linux Kernel Mailing List, Jan Beulich, Srivatsa Vaddagiri,
Jeremy Fitzhardinge, Avi Kivity, H. Peter Anvin,
Américo Wang, Eric Dumazet, Linux Virtualization
In-Reply-To: <92d21a90d4e27db4b46dbacd58ef67719acd0185.1289940821.git.jeremy.fitzhardinge@citrix.com>
> static inline int __ticket_spin_is_locked(arch_spinlock_t *lock)
> {
> - int tmp = ACCESS_ONCE(lock->slock);
> + struct __raw_tickets tmp = ACCESS_ONCE(lock->tickets);
>
> - return !!(((tmp >> TICKET_SHIFT) ^ tmp) & ((1 << TICKET_SHIFT) - 1));
> + return !!(tmp.tail ^ tmp.head);
Does it make sense to mask it here it here with TICKET_MASK as it was done before?
^ permalink raw reply
* Re: [PATCH] xen: fix non-ANSI function warning in irq.c
From: Konrad Rzeszutek Wilk @ 2011-01-11 15:15 UTC (permalink / raw)
To: Randy Dunlap; +Cc: virtualization, xen-devel, Jeremy Fitzhardinge
In-Reply-To: <20110108200036.20ab98c7.randy.dunlap@oracle.com>
On Sat, Jan 08, 2011 at 08:00:36PM -0800, Randy Dunlap wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
>
> Fix sparse warning for non-ANSI function declaration:
>
> arch/x86/xen/irq.c:129:30: warning: non-ANSI function declaration of function 'xen_init_irq_ops'
Thanks, queued up for rc1.
>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> ---
> arch/x86/xen/irq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- lnx0107.orig/arch/x86/xen/irq.c
> +++ lnx0107/arch/x86/xen/irq.c
> @@ -126,7 +126,7 @@ static const struct pv_irq_ops xen_irq_o
> #endif
> };
>
> -void __init xen_init_irq_ops()
> +void __init xen_init_irq_ops(void)
> {
> pv_irq_ops = xen_irq_ops;
> x86_init.irqs.intr_init = xen_init_IRQ;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Michael S. Tsirkin @ 2011-01-10 17:44 UTC (permalink / raw)
To: Milton Miller
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
virtualization
In-Reply-To: <virtio-pci-noroot@mdm.bga.com>
On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> We sometimes need to map between the virtio device and
> the given pci device. One such use is OS installer that
> gets the boot pci device from BIOS and needs to
> find the relevant block device. Since it can't,
> installation fails.
>
> Instead of creating a top-level devices/virtio-pci
> directory, create each device under the corresponding
> pci device node. Symlinks to all virtio-pci
> devices can be found under the pci driver link in
> bus/pci/drivers/virtio-pci/devices, and all virtio
> devices under drivers/bus/virtio/devices.
>
> Signed-off-by: Milton Miller <miltonm@bga.com>
Tested-by: "Daniel P. Berrange" <berrange@redhat.com>
> ---
>
> This is an alternative to the patch by Michael S. Tsirkin
> titled "virtio-pci: add softlinks between virtio and pci"
> https://patchwork.kernel.org/patch/454581/
>
> It creates simpler code, uses less memory, and should
> be even easier use by the installer as it won't have to
> know a virtio symlink to follow (just follow none).
>
> Compile tested only as I don't have kvm setup.
>
>
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index ef8d9d5..4fb5b2b 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
>
> MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
>
> -/* A PCI device has it's own struct device and so does a virtio device so
> - * we create a place for the virtio devices to show up in sysfs. I think it
> - * would make more sense for virtio to not insist on having it's own device. */
> -static struct device *virtio_pci_root;
> -
> /* Convert a generic virtio device to our structure */
> static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> {
> @@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> if (vp_dev == NULL)
> return -ENOMEM;
>
> - vp_dev->vdev.dev.parent = virtio_pci_root;
> + vp_dev->vdev.dev.parent = &pci_dev->dev;
> vp_dev->vdev.dev.release = virtio_pci_release_dev;
> vp_dev->vdev.config = &virtio_pci_config_ops;
> vp_dev->pci_dev = pci_dev;
> @@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
>
> static int __init virtio_pci_init(void)
> {
> - int err;
> -
> - virtio_pci_root = root_device_register("virtio-pci");
> - if (IS_ERR(virtio_pci_root))
> - return PTR_ERR(virtio_pci_root);
> -
> - err = pci_register_driver(&virtio_pci_driver);
> - if (err)
> - root_device_unregister(virtio_pci_root);
> -
> - return err;
> + return pci_register_driver(&virtio_pci_driver);
> }
>
> module_init(virtio_pci_init);
> @@ -735,7 +720,6 @@ module_init(virtio_pci_init);
> static void __exit virtio_pci_exit(void)
> {
> pci_unregister_driver(&virtio_pci_driver);
> - root_device_unregister(virtio_pci_root);
> }
>
> module_exit(virtio_pci_exit);
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Michael S. Tsirkin @ 2011-01-10 17:19 UTC (permalink / raw)
To: Gleb Natapov
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110171046.GB27292@redhat.com>
On Mon, Jan 10, 2011 at 07:10:46PM +0200, Gleb Natapov wrote:
> On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > We sometimes need to map between the virtio device and
> > the given pci device. One such use is OS installer that
> > gets the boot pci device from BIOS and needs to
> > find the relevant block device. Since it can't,
> > installation fails.
> >
> > Instead of creating a top-level devices/virtio-pci
> > directory, create each device under the corresponding
> > pci device node. Symlinks to all virtio-pci
> > devices can be found under the pci driver link in
> > bus/pci/drivers/virtio-pci/devices, and all virtio
> > devices under drivers/bus/virtio/devices.
> >
> > Signed-off-by: Milton Miller <miltonm@bga.com>
> Acked-by: Gleb Natapov <gleb@redhat.com>
Rusty, any comments?
We are going to tell the installer guys that this
is the interface, would be nice to know that the
patch is queued before we do.
> > ---
> >
> > This is an alternative to the patch by Michael S. Tsirkin
> > titled "virtio-pci: add softlinks between virtio and pci"
> > https://patchwork.kernel.org/patch/454581/
> >
> > It creates simpler code, uses less memory, and should
> > be even easier use by the installer as it won't have to
> > know a virtio symlink to follow (just follow none).
> >
> > Compile tested only as I don't have kvm setup.
> >
> >
> > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> > index ef8d9d5..4fb5b2b 100644
> > --- a/drivers/virtio/virtio_pci.c
> > +++ b/drivers/virtio/virtio_pci.c
> > @@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
> >
> > MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
> >
> > -/* A PCI device has it's own struct device and so does a virtio device so
> > - * we create a place for the virtio devices to show up in sysfs. I think it
> > - * would make more sense for virtio to not insist on having it's own device. */
> > -static struct device *virtio_pci_root;
> > -
> > /* Convert a generic virtio device to our structure */
> > static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> > {
> > @@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> > if (vp_dev == NULL)
> > return -ENOMEM;
> >
> > - vp_dev->vdev.dev.parent = virtio_pci_root;
> > + vp_dev->vdev.dev.parent = &pci_dev->dev;
> > vp_dev->vdev.dev.release = virtio_pci_release_dev;
> > vp_dev->vdev.config = &virtio_pci_config_ops;
> > vp_dev->pci_dev = pci_dev;
> > @@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
> >
> > static int __init virtio_pci_init(void)
> > {
> > - int err;
> > -
> > - virtio_pci_root = root_device_register("virtio-pci");
> > - if (IS_ERR(virtio_pci_root))
> > - return PTR_ERR(virtio_pci_root);
> > -
> > - err = pci_register_driver(&virtio_pci_driver);
> > - if (err)
> > - root_device_unregister(virtio_pci_root);
> > -
> > - return err;
> > + return pci_register_driver(&virtio_pci_driver);
> > }
> >
> > module_init(virtio_pci_init);
> > @@ -735,7 +720,6 @@ module_init(virtio_pci_init);
> > static void __exit virtio_pci_exit(void)
> > {
> > pci_unregister_driver(&virtio_pci_driver);
> > - root_device_unregister(virtio_pci_root);
> > }
> >
> > module_exit(virtio_pci_exit);
>
> --
> Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Gleb Natapov @ 2011-01-10 17:10 UTC (permalink / raw)
To: Milton Miller
Cc: Anthony Liguori, Michael S. Tsirkin, Jamie Lokier, Thomas Weber,
linux-kernel, virtualization
In-Reply-To: <virtio-pci-noroot@mdm.bga.com>
On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> We sometimes need to map between the virtio device and
> the given pci device. One such use is OS installer that
> gets the boot pci device from BIOS and needs to
> find the relevant block device. Since it can't,
> installation fails.
>
> Instead of creating a top-level devices/virtio-pci
> directory, create each device under the corresponding
> pci device node. Symlinks to all virtio-pci
> devices can be found under the pci driver link in
> bus/pci/drivers/virtio-pci/devices, and all virtio
> devices under drivers/bus/virtio/devices.
>
> Signed-off-by: Milton Miller <miltonm@bga.com>
Acked-by: Gleb Natapov <gleb@redhat.com>
> ---
>
> This is an alternative to the patch by Michael S. Tsirkin
> titled "virtio-pci: add softlinks between virtio and pci"
> https://patchwork.kernel.org/patch/454581/
>
> It creates simpler code, uses less memory, and should
> be even easier use by the installer as it won't have to
> know a virtio symlink to follow (just follow none).
>
> Compile tested only as I don't have kvm setup.
>
>
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index ef8d9d5..4fb5b2b 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
>
> MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
>
> -/* A PCI device has it's own struct device and so does a virtio device so
> - * we create a place for the virtio devices to show up in sysfs. I think it
> - * would make more sense for virtio to not insist on having it's own device. */
> -static struct device *virtio_pci_root;
> -
> /* Convert a generic virtio device to our structure */
> static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> {
> @@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> if (vp_dev == NULL)
> return -ENOMEM;
>
> - vp_dev->vdev.dev.parent = virtio_pci_root;
> + vp_dev->vdev.dev.parent = &pci_dev->dev;
> vp_dev->vdev.dev.release = virtio_pci_release_dev;
> vp_dev->vdev.config = &virtio_pci_config_ops;
> vp_dev->pci_dev = pci_dev;
> @@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
>
> static int __init virtio_pci_init(void)
> {
> - int err;
> -
> - virtio_pci_root = root_device_register("virtio-pci");
> - if (IS_ERR(virtio_pci_root))
> - return PTR_ERR(virtio_pci_root);
> -
> - err = pci_register_driver(&virtio_pci_driver);
> - if (err)
> - root_device_unregister(virtio_pci_root);
> -
> - return err;
> + return pci_register_driver(&virtio_pci_driver);
> }
>
> module_init(virtio_pci_init);
> @@ -735,7 +720,6 @@ module_init(virtio_pci_init);
> static void __exit virtio_pci_exit(void)
> {
> pci_unregister_driver(&virtio_pci_driver);
> - root_device_unregister(virtio_pci_root);
> }
>
> module_exit(virtio_pci_exit);
--
Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Michael S. Tsirkin @ 2011-01-10 14:19 UTC (permalink / raw)
To: Gleb Natapov
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110134050.GK2202@redhat.com>
On Mon, Jan 10, 2011 at 03:40:50PM +0200, Gleb Natapov wrote:
> On Mon, Jan 10, 2011 at 03:31:40PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 10, 2011 at 02:50:11PM +0200, Gleb Natapov wrote:
> > > On Mon, Jan 10, 2011 at 02:08:08PM +0200, Michael S. Tsirkin wrote:
> > > > On Mon, Jan 10, 2011 at 01:42:19PM +0200, Gleb Natapov wrote:
> > > > > On Mon, Jan 10, 2011 at 01:27:39PM +0200, Gleb Natapov wrote:
> > > > > > On Mon, Jan 10, 2011 at 01:22:05PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> > > > > > > > On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > > > > > > > > We sometimes need to map between the virtio device and
> > > > > > > > > > the given pci device. One such use is OS installer that
> > > > > > > > > > gets the boot pci device from BIOS and needs to
> > > > > > > > > > find the relevant block device. Since it can't,
> > > > > > > > > > installation fails.
> > > > > > > > > >
> > > > > > > > > > Instead of creating a top-level devices/virtio-pci
> > > > > > > > > > directory, create each device under the corresponding
> > > > > > > > > > pci device node. Symlinks to all virtio-pci
> > > > > > > > > > devices can be found under the pci driver link in
> > > > > > > > > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > > > > > > > > devices under drivers/bus/virtio/devices.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Milton Miller <miltonm@bga.com>
> > > > > > > > >
> > > > > > > > > OK, this works fine for me. I played with options to add compat
> > > > > > > > > softlinks under devices/virtio-pci but we still don't get exactly the
> > > > > > > > > same layout and since I don't think anyone actually uses them, it's
> > > > > > > > > probably ok to just to the simple thing.
> > > > > > > > >
> > > > > > > > > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > > > > >
> > > > > > > > > Rusty, since this help fix at least one user, any chance this can be put
> > > > > > > > > in 2.6.38? OK to backport to -stable?
> > > > > > > > >
> > > > > > > > > Gleb, could you try this out too?
> > > > > > > > >
> > > > > > > > With this patch if I have 3 virtio disks for a VM I get:
> > > > > > > > /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> > > > > > > > /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> > > > > > > > /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
> > > > > > > >
> > > > > > > > Number after virtio has no much sense. It either should be dropped at all
> > > > > > > > or be always zero in case we will support more then one virtio controller
> > > > > > > > per pci card. In that case each virtio controller will have directories
> > > > > > > > virtio0/virtio1/virtio2... under same pci device directory.
> > > > > > >
> > > > > > > Yes. But this is the bus name. It must be unique - all devices
> > > > > > > also appear under /sys/bus/virtio/devices.
> > > > > > >
> > > > > > It is very strange king of bus that is spread over several PCI devices :)
> > > > > > It doesn't make much sense IMHO, but I can leave with it.
> > > > > >
> > > > > I can't "leave" with it, but I can "live" with it.
> > > >
> > > > The virtio bus is an attempt to make as many applications as
> > > > possible work transparently on any virtio system,
> > > > be it lguest, s390 or pci. Arbitrary IDs is just a hint to the
> > > > applications 'don't rely on the name at all'.
> > > >
> > > Shouldn't sysfs directory reflect real device topology?
> >
> > What exactly is proposed here? Are you acking or nacking this patch?
> > Prefer the older version? Can send your own?
> >
> What exactly is proposed is to get rid of random number in virtio
> directory name. If this is complicated, as I said above, I can live
> with it as is.
Certainly more complicated than this patch:
core currently uses same dev_name everywhere.
> > > There are other buses AFAIK that connected differently on different
> > > HW. How virtio is special?
> > > What applications this allow to work transparently?
> >
> > I don't know which applications use the current sysfs topology
> > but the assumption that there are none and we can break it
> > arbitrarily seems a bit drastic.
> >
> > > --
> > > Gleb.
>
> --
> Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Gleb Natapov @ 2011-01-10 13:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110133140.GB15554@redhat.com>
On Mon, Jan 10, 2011 at 03:31:40PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 10, 2011 at 02:50:11PM +0200, Gleb Natapov wrote:
> > On Mon, Jan 10, 2011 at 02:08:08PM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Jan 10, 2011 at 01:42:19PM +0200, Gleb Natapov wrote:
> > > > On Mon, Jan 10, 2011 at 01:27:39PM +0200, Gleb Natapov wrote:
> > > > > On Mon, Jan 10, 2011 at 01:22:05PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> > > > > > > On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > > > > > > > We sometimes need to map between the virtio device and
> > > > > > > > > the given pci device. One such use is OS installer that
> > > > > > > > > gets the boot pci device from BIOS and needs to
> > > > > > > > > find the relevant block device. Since it can't,
> > > > > > > > > installation fails.
> > > > > > > > >
> > > > > > > > > Instead of creating a top-level devices/virtio-pci
> > > > > > > > > directory, create each device under the corresponding
> > > > > > > > > pci device node. Symlinks to all virtio-pci
> > > > > > > > > devices can be found under the pci driver link in
> > > > > > > > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > > > > > > > devices under drivers/bus/virtio/devices.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Milton Miller <miltonm@bga.com>
> > > > > > > >
> > > > > > > > OK, this works fine for me. I played with options to add compat
> > > > > > > > softlinks under devices/virtio-pci but we still don't get exactly the
> > > > > > > > same layout and since I don't think anyone actually uses them, it's
> > > > > > > > probably ok to just to the simple thing.
> > > > > > > >
> > > > > > > > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > > > >
> > > > > > > > Rusty, since this help fix at least one user, any chance this can be put
> > > > > > > > in 2.6.38? OK to backport to -stable?
> > > > > > > >
> > > > > > > > Gleb, could you try this out too?
> > > > > > > >
> > > > > > > With this patch if I have 3 virtio disks for a VM I get:
> > > > > > > /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> > > > > > > /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> > > > > > > /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
> > > > > > >
> > > > > > > Number after virtio has no much sense. It either should be dropped at all
> > > > > > > or be always zero in case we will support more then one virtio controller
> > > > > > > per pci card. In that case each virtio controller will have directories
> > > > > > > virtio0/virtio1/virtio2... under same pci device directory.
> > > > > >
> > > > > > Yes. But this is the bus name. It must be unique - all devices
> > > > > > also appear under /sys/bus/virtio/devices.
> > > > > >
> > > > > It is very strange king of bus that is spread over several PCI devices :)
> > > > > It doesn't make much sense IMHO, but I can leave with it.
> > > > >
> > > > I can't "leave" with it, but I can "live" with it.
> > >
> > > The virtio bus is an attempt to make as many applications as
> > > possible work transparently on any virtio system,
> > > be it lguest, s390 or pci. Arbitrary IDs is just a hint to the
> > > applications 'don't rely on the name at all'.
> > >
> > Shouldn't sysfs directory reflect real device topology?
>
> What exactly is proposed here? Are you acking or nacking this patch?
> Prefer the older version? Can send your own?
>
What exactly is proposed is to get rid of random number in virtio
directory name. If this is complicated, as I said above, I can live
with it as is.
> > There are other buses AFAIK that connected differently on different
> > HW. How virtio is special?
> > What applications this allow to work transparently?
>
> I don't know which applications use the current sysfs topology
> but the assumption that there are none and we can break it
> arbitrarily seems a bit drastic.
>
> > --
> > Gleb.
--
Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Michael S. Tsirkin @ 2011-01-10 13:31 UTC (permalink / raw)
To: Gleb Natapov
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110125011.GI2202@redhat.com>
On Mon, Jan 10, 2011 at 02:50:11PM +0200, Gleb Natapov wrote:
> On Mon, Jan 10, 2011 at 02:08:08PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 10, 2011 at 01:42:19PM +0200, Gleb Natapov wrote:
> > > On Mon, Jan 10, 2011 at 01:27:39PM +0200, Gleb Natapov wrote:
> > > > On Mon, Jan 10, 2011 at 01:22:05PM +0200, Michael S. Tsirkin wrote:
> > > > > On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> > > > > > On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > > > > > > We sometimes need to map between the virtio device and
> > > > > > > > the given pci device. One such use is OS installer that
> > > > > > > > gets the boot pci device from BIOS and needs to
> > > > > > > > find the relevant block device. Since it can't,
> > > > > > > > installation fails.
> > > > > > > >
> > > > > > > > Instead of creating a top-level devices/virtio-pci
> > > > > > > > directory, create each device under the corresponding
> > > > > > > > pci device node. Symlinks to all virtio-pci
> > > > > > > > devices can be found under the pci driver link in
> > > > > > > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > > > > > > devices under drivers/bus/virtio/devices.
> > > > > > > >
> > > > > > > > Signed-off-by: Milton Miller <miltonm@bga.com>
> > > > > > >
> > > > > > > OK, this works fine for me. I played with options to add compat
> > > > > > > softlinks under devices/virtio-pci but we still don't get exactly the
> > > > > > > same layout and since I don't think anyone actually uses them, it's
> > > > > > > probably ok to just to the simple thing.
> > > > > > >
> > > > > > > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > > >
> > > > > > > Rusty, since this help fix at least one user, any chance this can be put
> > > > > > > in 2.6.38? OK to backport to -stable?
> > > > > > >
> > > > > > > Gleb, could you try this out too?
> > > > > > >
> > > > > > With this patch if I have 3 virtio disks for a VM I get:
> > > > > > /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> > > > > > /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> > > > > > /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
> > > > > >
> > > > > > Number after virtio has no much sense. It either should be dropped at all
> > > > > > or be always zero in case we will support more then one virtio controller
> > > > > > per pci card. In that case each virtio controller will have directories
> > > > > > virtio0/virtio1/virtio2... under same pci device directory.
> > > > >
> > > > > Yes. But this is the bus name. It must be unique - all devices
> > > > > also appear under /sys/bus/virtio/devices.
> > > > >
> > > > It is very strange king of bus that is spread over several PCI devices :)
> > > > It doesn't make much sense IMHO, but I can leave with it.
> > > >
> > > I can't "leave" with it, but I can "live" with it.
> >
> > The virtio bus is an attempt to make as many applications as
> > possible work transparently on any virtio system,
> > be it lguest, s390 or pci. Arbitrary IDs is just a hint to the
> > applications 'don't rely on the name at all'.
> >
> Shouldn't sysfs directory reflect real device topology?
What exactly is proposed here? Are you acking or nacking this patch?
Prefer the older version? Can send your own?
> There are other buses AFAIK that connected differently on different
> HW. How virtio is special?
> What applications this allow to work transparently?
I don't know which applications use the current sysfs topology
but the assumption that there are none and we can break it
arbitrarily seems a bit drastic.
> --
> Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Gleb Natapov @ 2011-01-10 12:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110120808.GA15554@redhat.com>
On Mon, Jan 10, 2011 at 02:08:08PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 10, 2011 at 01:42:19PM +0200, Gleb Natapov wrote:
> > On Mon, Jan 10, 2011 at 01:27:39PM +0200, Gleb Natapov wrote:
> > > On Mon, Jan 10, 2011 at 01:22:05PM +0200, Michael S. Tsirkin wrote:
> > > > On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> > > > > On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > > > > > We sometimes need to map between the virtio device and
> > > > > > > the given pci device. One such use is OS installer that
> > > > > > > gets the boot pci device from BIOS and needs to
> > > > > > > find the relevant block device. Since it can't,
> > > > > > > installation fails.
> > > > > > >
> > > > > > > Instead of creating a top-level devices/virtio-pci
> > > > > > > directory, create each device under the corresponding
> > > > > > > pci device node. Symlinks to all virtio-pci
> > > > > > > devices can be found under the pci driver link in
> > > > > > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > > > > > devices under drivers/bus/virtio/devices.
> > > > > > >
> > > > > > > Signed-off-by: Milton Miller <miltonm@bga.com>
> > > > > >
> > > > > > OK, this works fine for me. I played with options to add compat
> > > > > > softlinks under devices/virtio-pci but we still don't get exactly the
> > > > > > same layout and since I don't think anyone actually uses them, it's
> > > > > > probably ok to just to the simple thing.
> > > > > >
> > > > > > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > >
> > > > > > Rusty, since this help fix at least one user, any chance this can be put
> > > > > > in 2.6.38? OK to backport to -stable?
> > > > > >
> > > > > > Gleb, could you try this out too?
> > > > > >
> > > > > With this patch if I have 3 virtio disks for a VM I get:
> > > > > /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> > > > > /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> > > > > /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
> > > > >
> > > > > Number after virtio has no much sense. It either should be dropped at all
> > > > > or be always zero in case we will support more then one virtio controller
> > > > > per pci card. In that case each virtio controller will have directories
> > > > > virtio0/virtio1/virtio2... under same pci device directory.
> > > >
> > > > Yes. But this is the bus name. It must be unique - all devices
> > > > also appear under /sys/bus/virtio/devices.
> > > >
> > > It is very strange king of bus that is spread over several PCI devices :)
> > > It doesn't make much sense IMHO, but I can leave with it.
> > >
> > I can't "leave" with it, but I can "live" with it.
>
> The virtio bus is an attempt to make as many applications as
> possible work transparently on any virtio system,
> be it lguest, s390 or pci. Arbitrary IDs is just a hint to the
> applications 'don't rely on the name at all'.
>
Shouldn't sysfs directory reflect real device topology? There are other
buses AFAIK that connected differently on different HW. How virtio is
special? What applications this allow to work transparently?
--
Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Michael S. Tsirkin @ 2011-01-10 12:08 UTC (permalink / raw)
To: Gleb Natapov
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110114219.GE2202@redhat.com>
On Mon, Jan 10, 2011 at 01:42:19PM +0200, Gleb Natapov wrote:
> On Mon, Jan 10, 2011 at 01:27:39PM +0200, Gleb Natapov wrote:
> > On Mon, Jan 10, 2011 at 01:22:05PM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> > > > On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > > > > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > > > > We sometimes need to map between the virtio device and
> > > > > > the given pci device. One such use is OS installer that
> > > > > > gets the boot pci device from BIOS and needs to
> > > > > > find the relevant block device. Since it can't,
> > > > > > installation fails.
> > > > > >
> > > > > > Instead of creating a top-level devices/virtio-pci
> > > > > > directory, create each device under the corresponding
> > > > > > pci device node. Symlinks to all virtio-pci
> > > > > > devices can be found under the pci driver link in
> > > > > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > > > > devices under drivers/bus/virtio/devices.
> > > > > >
> > > > > > Signed-off-by: Milton Miller <miltonm@bga.com>
> > > > >
> > > > > OK, this works fine for me. I played with options to add compat
> > > > > softlinks under devices/virtio-pci but we still don't get exactly the
> > > > > same layout and since I don't think anyone actually uses them, it's
> > > > > probably ok to just to the simple thing.
> > > > >
> > > > > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > > >
> > > > > Rusty, since this help fix at least one user, any chance this can be put
> > > > > in 2.6.38? OK to backport to -stable?
> > > > >
> > > > > Gleb, could you try this out too?
> > > > >
> > > > With this patch if I have 3 virtio disks for a VM I get:
> > > > /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> > > > /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> > > > /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
> > > >
> > > > Number after virtio has no much sense. It either should be dropped at all
> > > > or be always zero in case we will support more then one virtio controller
> > > > per pci card. In that case each virtio controller will have directories
> > > > virtio0/virtio1/virtio2... under same pci device directory.
> > >
> > > Yes. But this is the bus name. It must be unique - all devices
> > > also appear under /sys/bus/virtio/devices.
> > >
> > It is very strange king of bus that is spread over several PCI devices :)
> > It doesn't make much sense IMHO, but I can leave with it.
> >
> I can't "leave" with it, but I can "live" with it.
The virtio bus is an attempt to make as many applications as
possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the
applications 'don't rely on the name at all'.
> --
> Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Gleb Natapov @ 2011-01-10 11:42 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110112739.GD2202@redhat.com>
On Mon, Jan 10, 2011 at 01:27:39PM +0200, Gleb Natapov wrote:
> On Mon, Jan 10, 2011 at 01:22:05PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> > > On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > > > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > > > We sometimes need to map between the virtio device and
> > > > > the given pci device. One such use is OS installer that
> > > > > gets the boot pci device from BIOS and needs to
> > > > > find the relevant block device. Since it can't,
> > > > > installation fails.
> > > > >
> > > > > Instead of creating a top-level devices/virtio-pci
> > > > > directory, create each device under the corresponding
> > > > > pci device node. Symlinks to all virtio-pci
> > > > > devices can be found under the pci driver link in
> > > > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > > > devices under drivers/bus/virtio/devices.
> > > > >
> > > > > Signed-off-by: Milton Miller <miltonm@bga.com>
> > > >
> > > > OK, this works fine for me. I played with options to add compat
> > > > softlinks under devices/virtio-pci but we still don't get exactly the
> > > > same layout and since I don't think anyone actually uses them, it's
> > > > probably ok to just to the simple thing.
> > > >
> > > > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > > >
> > > > Rusty, since this help fix at least one user, any chance this can be put
> > > > in 2.6.38? OK to backport to -stable?
> > > >
> > > > Gleb, could you try this out too?
> > > >
> > > With this patch if I have 3 virtio disks for a VM I get:
> > > /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> > > /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> > > /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
> > >
> > > Number after virtio has no much sense. It either should be dropped at all
> > > or be always zero in case we will support more then one virtio controller
> > > per pci card. In that case each virtio controller will have directories
> > > virtio0/virtio1/virtio2... under same pci device directory.
> >
> > Yes. But this is the bus name. It must be unique - all devices
> > also appear under /sys/bus/virtio/devices.
> >
> It is very strange king of bus that is spread over several PCI devices :)
> It doesn't make much sense IMHO, but I can leave with it.
>
I can't "leave" with it, but I can "live" with it.
--
Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Gleb Natapov @ 2011-01-10 11:27 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110112205.GC12065@redhat.com>
On Mon, Jan 10, 2011 at 01:22:05PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> > On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > > We sometimes need to map between the virtio device and
> > > > the given pci device. One such use is OS installer that
> > > > gets the boot pci device from BIOS and needs to
> > > > find the relevant block device. Since it can't,
> > > > installation fails.
> > > >
> > > > Instead of creating a top-level devices/virtio-pci
> > > > directory, create each device under the corresponding
> > > > pci device node. Symlinks to all virtio-pci
> > > > devices can be found under the pci driver link in
> > > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > > devices under drivers/bus/virtio/devices.
> > > >
> > > > Signed-off-by: Milton Miller <miltonm@bga.com>
> > >
> > > OK, this works fine for me. I played with options to add compat
> > > softlinks under devices/virtio-pci but we still don't get exactly the
> > > same layout and since I don't think anyone actually uses them, it's
> > > probably ok to just to the simple thing.
> > >
> > > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > >
> > > Rusty, since this help fix at least one user, any chance this can be put
> > > in 2.6.38? OK to backport to -stable?
> > >
> > > Gleb, could you try this out too?
> > >
> > With this patch if I have 3 virtio disks for a VM I get:
> > /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> > /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> > /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
> >
> > Number after virtio has no much sense. It either should be dropped at all
> > or be always zero in case we will support more then one virtio controller
> > per pci card. In that case each virtio controller will have directories
> > virtio0/virtio1/virtio2... under same pci device directory.
>
> Yes. But this is the bus name. It must be unique - all devices
> also appear under /sys/bus/virtio/devices.
>
It is very strange king of bus that is spread over several PCI devices :)
It doesn't make much sense IMHO, but I can leave with it.
> > > > ---
> > > >
> > > > This is an alternative to the patch by Michael S. Tsirkin
> > > > titled "virtio-pci: add softlinks between virtio and pci"
> > > > https://patchwork.kernel.org/patch/454581/
> > > >
> > > > It creates simpler code, uses less memory, and should
> > > > be even easier use by the installer as it won't have to
> > > > know a virtio symlink to follow (just follow none).
> > > >
> > > > Compile tested only as I don't have kvm setup.
> > > >
> > > >
> > > > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> > > > index ef8d9d5..4fb5b2b 100644
> > > > --- a/drivers/virtio/virtio_pci.c
> > > > +++ b/drivers/virtio/virtio_pci.c
> > > > @@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
> > > >
> > > > MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
> > > >
> > > > -/* A PCI device has it's own struct device and so does a virtio device so
> > > > - * we create a place for the virtio devices to show up in sysfs. I think it
> > > > - * would make more sense for virtio to not insist on having it's own device. */
> > > > -static struct device *virtio_pci_root;
> > > > -
> > > > /* Convert a generic virtio device to our structure */
> > > > static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> > > > {
> > > > @@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> > > > if (vp_dev == NULL)
> > > > return -ENOMEM;
> > > >
> > > > - vp_dev->vdev.dev.parent = virtio_pci_root;
> > > > + vp_dev->vdev.dev.parent = &pci_dev->dev;
> > > > vp_dev->vdev.dev.release = virtio_pci_release_dev;
> > > > vp_dev->vdev.config = &virtio_pci_config_ops;
> > > > vp_dev->pci_dev = pci_dev;
> > > > @@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
> > > >
> > > > static int __init virtio_pci_init(void)
> > > > {
> > > > - int err;
> > > > -
> > > > - virtio_pci_root = root_device_register("virtio-pci");
> > > > - if (IS_ERR(virtio_pci_root))
> > > > - return PTR_ERR(virtio_pci_root);
> > > > -
> > > > - err = pci_register_driver(&virtio_pci_driver);
> > > > - if (err)
> > > > - root_device_unregister(virtio_pci_root);
> > > > -
> > > > - return err;
> > > > + return pci_register_driver(&virtio_pci_driver);
> > > > }
> > > >
> > > > module_init(virtio_pci_init);
> > > > @@ -735,7 +720,6 @@ module_init(virtio_pci_init);
> > > > static void __exit virtio_pci_exit(void)
> > > > {
> > > > pci_unregister_driver(&virtio_pci_driver);
> > > > - root_device_unregister(virtio_pci_root);
> > > > }
> > > >
> > > > module_exit(virtio_pci_exit);
> >
> > --
> > Gleb.
--
Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Michael S. Tsirkin @ 2011-01-10 11:22 UTC (permalink / raw)
To: Gleb Natapov
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110110111859.GC2202@redhat.com>
On Mon, Jan 10, 2011 at 01:18:59PM +0200, Gleb Natapov wrote:
> On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > > We sometimes need to map between the virtio device and
> > > the given pci device. One such use is OS installer that
> > > gets the boot pci device from BIOS and needs to
> > > find the relevant block device. Since it can't,
> > > installation fails.
> > >
> > > Instead of creating a top-level devices/virtio-pci
> > > directory, create each device under the corresponding
> > > pci device node. Symlinks to all virtio-pci
> > > devices can be found under the pci driver link in
> > > bus/pci/drivers/virtio-pci/devices, and all virtio
> > > devices under drivers/bus/virtio/devices.
> > >
> > > Signed-off-by: Milton Miller <miltonm@bga.com>
> >
> > OK, this works fine for me. I played with options to add compat
> > softlinks under devices/virtio-pci but we still don't get exactly the
> > same layout and since I don't think anyone actually uses them, it's
> > probably ok to just to the simple thing.
> >
> > Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > Rusty, since this help fix at least one user, any chance this can be put
> > in 2.6.38? OK to backport to -stable?
> >
> > Gleb, could you try this out too?
> >
> With this patch if I have 3 virtio disks for a VM I get:
> /sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
> /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
> /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
>
> Number after virtio has no much sense. It either should be dropped at all
> or be always zero in case we will support more then one virtio controller
> per pci card. In that case each virtio controller will have directories
> virtio0/virtio1/virtio2... under same pci device directory.
Yes. But this is the bus name. It must be unique - all devices
also appear under /sys/bus/virtio/devices.
> > > ---
> > >
> > > This is an alternative to the patch by Michael S. Tsirkin
> > > titled "virtio-pci: add softlinks between virtio and pci"
> > > https://patchwork.kernel.org/patch/454581/
> > >
> > > It creates simpler code, uses less memory, and should
> > > be even easier use by the installer as it won't have to
> > > know a virtio symlink to follow (just follow none).
> > >
> > > Compile tested only as I don't have kvm setup.
> > >
> > >
> > > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> > > index ef8d9d5..4fb5b2b 100644
> > > --- a/drivers/virtio/virtio_pci.c
> > > +++ b/drivers/virtio/virtio_pci.c
> > > @@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
> > >
> > > MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
> > >
> > > -/* A PCI device has it's own struct device and so does a virtio device so
> > > - * we create a place for the virtio devices to show up in sysfs. I think it
> > > - * would make more sense for virtio to not insist on having it's own device. */
> > > -static struct device *virtio_pci_root;
> > > -
> > > /* Convert a generic virtio device to our structure */
> > > static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> > > {
> > > @@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> > > if (vp_dev == NULL)
> > > return -ENOMEM;
> > >
> > > - vp_dev->vdev.dev.parent = virtio_pci_root;
> > > + vp_dev->vdev.dev.parent = &pci_dev->dev;
> > > vp_dev->vdev.dev.release = virtio_pci_release_dev;
> > > vp_dev->vdev.config = &virtio_pci_config_ops;
> > > vp_dev->pci_dev = pci_dev;
> > > @@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
> > >
> > > static int __init virtio_pci_init(void)
> > > {
> > > - int err;
> > > -
> > > - virtio_pci_root = root_device_register("virtio-pci");
> > > - if (IS_ERR(virtio_pci_root))
> > > - return PTR_ERR(virtio_pci_root);
> > > -
> > > - err = pci_register_driver(&virtio_pci_driver);
> > > - if (err)
> > > - root_device_unregister(virtio_pci_root);
> > > -
> > > - return err;
> > > + return pci_register_driver(&virtio_pci_driver);
> > > }
> > >
> > > module_init(virtio_pci_init);
> > > @@ -735,7 +720,6 @@ module_init(virtio_pci_init);
> > > static void __exit virtio_pci_exit(void)
> > > {
> > > pci_unregister_driver(&virtio_pci_driver);
> > > - root_device_unregister(virtio_pci_root);
> > > }
> > >
> > > module_exit(virtio_pci_exit);
>
> --
> Gleb.
^ permalink raw reply
* Re: [PATCH] virtio: remove virtio-pci root device
From: Gleb Natapov @ 2011-01-10 11:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
Milton Miller, virtualization
In-Reply-To: <20110109151821.GA9063@redhat.com>
On Sun, Jan 09, 2011 at 05:18:21PM +0200, Michael S. Tsirkin wrote:
> On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> > We sometimes need to map between the virtio device and
> > the given pci device. One such use is OS installer that
> > gets the boot pci device from BIOS and needs to
> > find the relevant block device. Since it can't,
> > installation fails.
> >
> > Instead of creating a top-level devices/virtio-pci
> > directory, create each device under the corresponding
> > pci device node. Symlinks to all virtio-pci
> > devices can be found under the pci driver link in
> > bus/pci/drivers/virtio-pci/devices, and all virtio
> > devices under drivers/bus/virtio/devices.
> >
> > Signed-off-by: Milton Miller <miltonm@bga.com>
>
> OK, this works fine for me. I played with options to add compat
> softlinks under devices/virtio-pci but we still don't get exactly the
> same layout and since I don't think anyone actually uses them, it's
> probably ok to just to the simple thing.
>
> Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Rusty, since this help fix at least one user, any chance this can be put
> in 2.6.38? OK to backport to -stable?
>
> Gleb, could you try this out too?
>
With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda
/sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb
/sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc
Number after virtio has no much sense. It either should be dropped at all
or be always zero in case we will support more then one virtio controller
per pci card. In that case each virtio controller will have directories
virtio0/virtio1/virtio2... under same pci device directory.
> > ---
> >
> > This is an alternative to the patch by Michael S. Tsirkin
> > titled "virtio-pci: add softlinks between virtio and pci"
> > https://patchwork.kernel.org/patch/454581/
> >
> > It creates simpler code, uses less memory, and should
> > be even easier use by the installer as it won't have to
> > know a virtio symlink to follow (just follow none).
> >
> > Compile tested only as I don't have kvm setup.
> >
> >
> > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> > index ef8d9d5..4fb5b2b 100644
> > --- a/drivers/virtio/virtio_pci.c
> > +++ b/drivers/virtio/virtio_pci.c
> > @@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
> >
> > MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
> >
> > -/* A PCI device has it's own struct device and so does a virtio device so
> > - * we create a place for the virtio devices to show up in sysfs. I think it
> > - * would make more sense for virtio to not insist on having it's own device. */
> > -static struct device *virtio_pci_root;
> > -
> > /* Convert a generic virtio device to our structure */
> > static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> > {
> > @@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> > if (vp_dev == NULL)
> > return -ENOMEM;
> >
> > - vp_dev->vdev.dev.parent = virtio_pci_root;
> > + vp_dev->vdev.dev.parent = &pci_dev->dev;
> > vp_dev->vdev.dev.release = virtio_pci_release_dev;
> > vp_dev->vdev.config = &virtio_pci_config_ops;
> > vp_dev->pci_dev = pci_dev;
> > @@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
> >
> > static int __init virtio_pci_init(void)
> > {
> > - int err;
> > -
> > - virtio_pci_root = root_device_register("virtio-pci");
> > - if (IS_ERR(virtio_pci_root))
> > - return PTR_ERR(virtio_pci_root);
> > -
> > - err = pci_register_driver(&virtio_pci_driver);
> > - if (err)
> > - root_device_unregister(virtio_pci_root);
> > -
> > - return err;
> > + return pci_register_driver(&virtio_pci_driver);
> > }
> >
> > module_init(virtio_pci_init);
> > @@ -735,7 +720,6 @@ module_init(virtio_pci_init);
> > static void __exit virtio_pci_exit(void)
> > {
> > pci_unregister_driver(&virtio_pci_driver);
> > - root_device_unregister(virtio_pci_root);
> > }
> >
> > module_exit(virtio_pci_exit);
--
Gleb.
^ permalink raw reply
* Re: Flow Control and Port Mirroring Revisited
From: Simon Horman @ 2011-01-10 9:31 UTC (permalink / raw)
To: Jesse Gross
Cc: Eric Dumazet, Rusty Russell, virtualization, dev, virtualization,
netdev, kvm, Michael S. Tsirkin
In-Reply-To: <20110107012356.GA1257@verge.net.au>
On Fri, Jan 07, 2011 at 10:23:58AM +0900, Simon Horman wrote:
> On Thu, Jan 06, 2011 at 05:38:01PM -0500, Jesse Gross wrote:
>
> [ snip ]
> >
> > I know that everyone likes a nice netperf result but I agree with
> > Michael that this probably isn't the right question to be asking. I
> > don't think that socket buffers are a real solution to the flow
> > control problem: they happen to provide that functionality but it's
> > more of a side effect than anything. It's just that the amount of
> > memory consumed by packets in the queue(s) doesn't really have any
> > implicit meaning for flow control (think multiple physical adapters,
> > all with the same speed instead of a virtual device and a physical
> > device with wildly different speeds). The analog in the physical
> > world that you're looking for would be Ethernet flow control.
> > Obviously, if the question is limiting CPU or memory consumption then
> > that's a different story.
>
> Point taken. I will see if I can control CPU (and thus memory) consumption
> using cgroups and/or tc.
I have found that I can successfully control the throughput using
the following techniques
1) Place a tc egress filter on dummy0
2) Use ovs-ofctl to add a flow that sends skbs to dummy0 and then eth1,
this is effectively the same as one of my hacks to the datapath
that I mentioned in an earlier mail. The result is that eth1
"paces" the connection.
3) 2) + place a tc egress filter on eth1
Which mostly makes sense to me although I am a little confused about
why 1) needs a filter on dummy0 (a filter on eth1 has no effect)
but 3) needs a filter on eth1 (a filter on dummy0 has no effect,
even if the skb is sent to dummy0 last.
I also had some limited success using CPU cgroups, though obviously
that targets CPU usage and thus the effect on throughput is fairly course.
In short, its a useful technique but not one that bares further
discussion here.
^ permalink raw reply
* [PATCH] vhost: fix signed/unsigned comparison
From: Michael S. Tsirkin @ 2011-01-10 8:03 UTC (permalink / raw)
To: Michael S. Tsirkin, kvm, virtualization, netdev, linux-kernel,
Jes.Sorensen, stable
To detect that a sequence number is done, we are doing math on unsigned
integers so the result is unsigned too. Not what was intended for the <=
comparison. The result is user stuck forever in flush call.
Convert to int to fix this.
Further, get rid of ({}) to make code clearer.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I've put this on my vhost.git tree.
drivers/vhost/vhost.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 159c77a..522b8fc 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -98,22 +98,26 @@ void vhost_poll_stop(struct vhost_poll *poll)
remove_wait_queue(poll->wqh, &poll->wait);
}
+static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
+ unsigned seq)
+{
+ int left;
+ spin_lock_irq(&dev->work_lock);
+ left = seq - work->done_seq;
+ spin_unlock_irq(&dev->work_lock);
+ return left <= 0;
+}
+
static void vhost_work_flush(struct vhost_dev *dev, struct vhost_work *work)
{
unsigned seq;
- int left;
int flushing;
spin_lock_irq(&dev->work_lock);
seq = work->queue_seq;
work->flushing++;
spin_unlock_irq(&dev->work_lock);
- wait_event(work->done, ({
- spin_lock_irq(&dev->work_lock);
- left = seq - work->done_seq <= 0;
- spin_unlock_irq(&dev->work_lock);
- left;
- }));
+ wait_event(work->done, vhost_work_seq_done(dev, work, seq));
spin_lock_irq(&dev->work_lock);
flushing = --work->flushing;
spin_unlock_irq(&dev->work_lock);
--
1.7.3.2.91.g446ac
^ permalink raw reply related
* Re: [PATCH] virtio: remove virtio-pci root device
From: Michael S. Tsirkin @ 2011-01-09 15:18 UTC (permalink / raw)
To: Milton Miller
Cc: Anthony Liguori, Jamie Lokier, Thomas Weber, linux-kernel,
virtualization
In-Reply-To: <virtio-pci-noroot@mdm.bga.com>
On Fri, Jan 07, 2011 at 02:55:06AM -0600, Milton Miller wrote:
> We sometimes need to map between the virtio device and
> the given pci device. One such use is OS installer that
> gets the boot pci device from BIOS and needs to
> find the relevant block device. Since it can't,
> installation fails.
>
> Instead of creating a top-level devices/virtio-pci
> directory, create each device under the corresponding
> pci device node. Symlinks to all virtio-pci
> devices can be found under the pci driver link in
> bus/pci/drivers/virtio-pci/devices, and all virtio
> devices under drivers/bus/virtio/devices.
>
> Signed-off-by: Milton Miller <miltonm@bga.com>
OK, this works fine for me. I played with options to add compat
softlinks under devices/virtio-pci but we still don't get exactly the
same layout and since I don't think anyone actually uses them, it's
probably ok to just to the simple thing.
Tested/Acked-by: Michael S. Tsirkin <mst@redhat.com>
Rusty, since this help fix at least one user, any chance this can be put
in 2.6.38? OK to backport to -stable?
Gleb, could you try this out too?
> ---
>
> This is an alternative to the patch by Michael S. Tsirkin
> titled "virtio-pci: add softlinks between virtio and pci"
> https://patchwork.kernel.org/patch/454581/
>
> It creates simpler code, uses less memory, and should
> be even easier use by the installer as it won't have to
> know a virtio symlink to follow (just follow none).
>
> Compile tested only as I don't have kvm setup.
>
>
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index ef8d9d5..4fb5b2b 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
>
> MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
>
> -/* A PCI device has it's own struct device and so does a virtio device so
> - * we create a place for the virtio devices to show up in sysfs. I think it
> - * would make more sense for virtio to not insist on having it's own device. */
> -static struct device *virtio_pci_root;
> -
> /* Convert a generic virtio device to our structure */
> static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
> {
> @@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
> if (vp_dev == NULL)
> return -ENOMEM;
>
> - vp_dev->vdev.dev.parent = virtio_pci_root;
> + vp_dev->vdev.dev.parent = &pci_dev->dev;
> vp_dev->vdev.dev.release = virtio_pci_release_dev;
> vp_dev->vdev.config = &virtio_pci_config_ops;
> vp_dev->pci_dev = pci_dev;
> @@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
>
> static int __init virtio_pci_init(void)
> {
> - int err;
> -
> - virtio_pci_root = root_device_register("virtio-pci");
> - if (IS_ERR(virtio_pci_root))
> - return PTR_ERR(virtio_pci_root);
> -
> - err = pci_register_driver(&virtio_pci_driver);
> - if (err)
> - root_device_unregister(virtio_pci_root);
> -
> - return err;
> + return pci_register_driver(&virtio_pci_driver);
> }
>
> module_init(virtio_pci_init);
> @@ -735,7 +720,6 @@ module_init(virtio_pci_init);
> static void __exit virtio_pci_exit(void)
> {
> pci_unregister_driver(&virtio_pci_driver);
> - root_device_unregister(virtio_pci_root);
> }
>
> module_exit(virtio_pci_exit);
^ permalink raw reply
* [PATCH] xen: fix non-ANSI function warning in irq.c
From: Randy Dunlap @ 2011-01-09 4:00 UTC (permalink / raw)
To: virtualization, xen-devel; +Cc: Jeremy Fitzhardinge
From: Randy Dunlap <randy.dunlap@oracle.com>
Fix sparse warning for non-ANSI function declaration:
arch/x86/xen/irq.c:129:30: warning: non-ANSI function declaration of function 'xen_init_irq_ops'
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
arch/x86/xen/irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- lnx0107.orig/arch/x86/xen/irq.c
+++ lnx0107/arch/x86/xen/irq.c
@@ -126,7 +126,7 @@ static const struct pv_irq_ops xen_irq_o
#endif
};
-void __init xen_init_irq_ops()
+void __init xen_init_irq_ops(void)
{
pv_irq_ops = xen_irq_ops;
x86_init.irqs.intr_init = xen_init_IRQ;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox