* Re: [PATCH 0/5] Add vhost-blk support
From: Michael S. Tsirkin @ 2012-07-17 11:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: linux-aio, kvm, linux-kernel, virtualization, Benjamin LaHaise,
Alexander Viro, linux-fsdevel, Paolo Bonzini
In-Reply-To: <CAJSP0QW0tPV3WASGOBRE4hL=8zzSFQ6T=bHbEwMAzmHR9wnwJg@mail.gmail.com>
On Tue, Jul 17, 2012 at 12:11:15PM +0100, Stefan Hajnoczi wrote:
> On Tue, Jul 17, 2012 at 10:21 AM, Asias He <asias@redhat.com> wrote:
> > On 07/17/2012 04:52 PM, Paolo Bonzini wrote:
> >>
> >> Il 17/07/2012 10:29, Asias He ha scritto:
> >>>
> >>> So, vhost-blk at least saves ~6 syscalls for us in each request.
> >>
> >>
> >> Are they really 6? If I/O is coalesced by a factor of 3, for example
> >> (i.e. each exit processes 3 requests), it's really 2 syscalls per request.
> >
> >
> > Well. I am counting the number of syscalls in one notify and response
> > process. Sure the IO can be coalesced.
>
> Linux AIO also supports batching in io_submit() and io_getevents().
> Depending on the request pattern in the vring when you process it, you
> should be able to do better than 1 set of syscalls per host I/O
> request.
>
> Are you taking advantage of that at the moment in your userspace benchmark?
>
> Stefan
Injecting an interrupt directly from kernel bypasses two context switches.
Yes some worloads can coalesce interrupts efficiently but others can't.
It is not really hard to speculate more.
Personally I don't understand where all this speculation leads us.
Are you guys disputing the measurements posted? If not would not
it be better if discussion focused on the amount of extra code versus
measured gain?
--
MST
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Stefan Hajnoczi @ 2012-07-17 11:11 UTC (permalink / raw)
To: Asias He
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel, Paolo Bonzini
In-Reply-To: <50052E7E.6020100@redhat.com>
On Tue, Jul 17, 2012 at 10:21 AM, Asias He <asias@redhat.com> wrote:
> On 07/17/2012 04:52 PM, Paolo Bonzini wrote:
>>
>> Il 17/07/2012 10:29, Asias He ha scritto:
>>>
>>> So, vhost-blk at least saves ~6 syscalls for us in each request.
>>
>>
>> Are they really 6? If I/O is coalesced by a factor of 3, for example
>> (i.e. each exit processes 3 requests), it's really 2 syscalls per request.
>
>
> Well. I am counting the number of syscalls in one notify and response
> process. Sure the IO can be coalesced.
Linux AIO also supports batching in io_submit() and io_getevents().
Depending on the request pattern in the vring when you process it, you
should be able to do better than 1 set of syscalls per host I/O
request.
Are you taking advantage of that at the moment in your userspace benchmark?
Stefan
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Michael S. Tsirkin @ 2012-07-17 11:09 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-aio, target-devel, kvm, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, Anthony Liguori, linux-fsdevel
In-Reply-To: <500544DF.3050400@redhat.com>
On Tue, Jul 17, 2012 at 12:56:31PM +0200, Paolo Bonzini wrote:
> Il 17/07/2012 12:49, Michael S. Tsirkin ha scritto:
> >> Ok, that would make more sense. One difference between vhost-blk and
> >> vhost-net is that for vhost-blk there are also management actions that
> >> would trigger the switch, for example a live snapshot.
> >> So a prerequisite for vhost-blk would be that it is possible to disable
> >> it on the fly while the VM is running, as soon as all in-flight I/O is
> >> completed.
> >
> > It applies for vhost-net too. For example if you bring link down,
> > we switch to userspace. So vhost-net supports this switch on the fly.
>
> Cool.
>
> >> (Note that, however, this is not possible for vhost-scsi, because it
> >> really exposes different hardware to the guest. It must not happen that
> >> a kernel upgrade or downgrade toggles between userspace SCSI and
> >> vhost-scsi, for example).
> >
> > I would say this is not a prerequisite for merging in qemu.
> > It might be a required feature for production but it
> > is also solvable at the management level.
>
> I'm thinking of the level interrupts here. You cannot make a change in
> the guest, and have it do completely unrelated changes the hardware that
> the guest sees.
Absolutely.
So the right thing for vhost-scsi might be to just support level
(equivalent for "force" flag in vhost-net).
We don't in vhost-net because it is triggered by some old guests
but all virtio-scsi guests use MSI so level is just a spec compatibility
issue.
We might also gain kernel support for level at some point.
> >>>> having to
> >>>> support the API; having to handle transition from one more thing when
> >>>> something better comes out.
> >>>
> >>> Well this is true for any code. If the limited featureset which
> >>> vhost-blk can accelerate is something many people use, then accelerating
> >>> by 5-15% might outweight support costs.
> >>
> >> It is definitely what people use if they are interested in performance.
> >
> > In that case it seems to me we should stop using the feature set as
> > an argument and focus on whether the extra code is worth the 5-15% gain.
> > No one seems to have commented on that so everyone on list thinks that
> > aspect is OK?
>
> I would like to see a breakdown of _where_ the 5-15% lies, something
> like http://www.linux-kvm.org/page/Virtio/Block/Latency.
Yes but I think it's also nice to have. It's hard to argue IMO that
virtio as kernel interface cuts out some overhead.
> > Kernel merge windows is coming up and I would like to see whether
> > any of vhost-blk / vhost-scsi is going to be actually used by userspace.
> > I guess we could tag it for staging but would be nice to avoid that.
>
> Staging would be fine by me for both vhost-blk and vhost-scsi.
>
> Paolo
The reason I say staging is because there seems to be a deadlock
where userspace waits for kernel to merge a driver and
kernel does not want to commit to an ABI that will then
go unused.
So even if it gets tagged as staging it would only
make sense for it to stay there for one cycle. And then either
get removed if no userspace materializes or lose staging tag
if it does.
--
MST
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Paolo Bonzini @ 2012-07-17 10:56 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-aio, target-devel, kvm, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, Anthony Liguori, linux-fsdevel
In-Reply-To: <20120717104920.GJ7949@redhat.com>
Il 17/07/2012 12:49, Michael S. Tsirkin ha scritto:
>> Ok, that would make more sense. One difference between vhost-blk and
>> vhost-net is that for vhost-blk there are also management actions that
>> would trigger the switch, for example a live snapshot.
>> So a prerequisite for vhost-blk would be that it is possible to disable
>> it on the fly while the VM is running, as soon as all in-flight I/O is
>> completed.
>
> It applies for vhost-net too. For example if you bring link down,
> we switch to userspace. So vhost-net supports this switch on the fly.
Cool.
>> (Note that, however, this is not possible for vhost-scsi, because it
>> really exposes different hardware to the guest. It must not happen that
>> a kernel upgrade or downgrade toggles between userspace SCSI and
>> vhost-scsi, for example).
>
> I would say this is not a prerequisite for merging in qemu.
> It might be a required feature for production but it
> is also solvable at the management level.
I'm thinking of the level interrupts here. You cannot make a change in
the guest, and have it do completely unrelated changes the hardware that
the guest sees.
>>>> having to
>>>> support the API; having to handle transition from one more thing when
>>>> something better comes out.
>>>
>>> Well this is true for any code. If the limited featureset which
>>> vhost-blk can accelerate is something many people use, then accelerating
>>> by 5-15% might outweight support costs.
>>
>> It is definitely what people use if they are interested in performance.
>
> In that case it seems to me we should stop using the feature set as
> an argument and focus on whether the extra code is worth the 5-15% gain.
> No one seems to have commented on that so everyone on list thinks that
> aspect is OK?
I would like to see a breakdown of _where_ the 5-15% lies, something
like http://www.linux-kvm.org/page/Virtio/Block/Latency.
> Kernel merge windows is coming up and I would like to see whether
> any of vhost-blk / vhost-scsi is going to be actually used by userspace.
> I guess we could tag it for staging but would be nice to avoid that.
Staging would be fine by me for both vhost-blk and vhost-scsi.
Paolo
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Michael S. Tsirkin @ 2012-07-17 10:49 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-aio, target-devel, kvm, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, Anthony Liguori, linux-fsdevel
In-Reply-To: <50053B09.2060703@redhat.com>
On Tue, Jul 17, 2012 at 12:14:33PM +0200, Paolo Bonzini wrote:
> Il 17/07/2012 11:45, Michael S. Tsirkin ha scritto:
> >> So it begs the question, is it going to be used in production, or just a
> >> useful reference tool?
> >
> > Sticking to raw already makes virtio-blk faster, doesn't it?
> > In that vhost-blk looks to me like just another optimization option.
> > Ideally I think user just should not care where do we handle virtio:
> > in-kernel or in userspace. One can imagine it being enabled/disabled
> > automatically if none of the features unsupported by it are used.
>
> Ok, that would make more sense. One difference between vhost-blk and
> vhost-net is that for vhost-blk there are also management actions that
> would trigger the switch, for example a live snapshot.
> So a prerequisite for vhost-blk would be that it is possible to disable
> it on the fly while the VM is running, as soon as all in-flight I/O is
> completed.
It applies for vhost-net too. For example if you bring link down,
we switch to userspace. So vhost-net supports this switch on the fly.
> (Note that, however, this is not possible for vhost-scsi,
> because it
> really exposes different hardware to the guest. It must not happen that
> a kernel upgrade or downgrade toggles between userspace SCSI and
> vhost-scsi, for example).
I would say this is not a prerequisite for merging in qemu.
It might be a required feature for production but it
is also solvable at the management level.
Imagine an "enable-live-snapshots" flag in libvirt, on by default.
Can only be changed while guest is down. If you turn it off,
you get a bit more speed since vhost-blk/vhost-scsi gets enabled.
Also pls note that a backend can support live snapshots.
If it does libvirt thinkably could detect that
and enable vhost-scsi even with enable-live-snapshots on.
> >> having to
> >> support the API; having to handle transition from one more thing when
> >> something better comes out.
> >
> > Well this is true for any code. If the limited featureset which
> > vhost-blk can accelerate is something many people use, then accelerating
> > by 5-15% might outweight support costs.
>
> It is definitely what people use if they are interested in performance.
>
> Paolo
In that case it seems to me we should stop using the featureset as
an argument and focus on whether the extra code is worth the 5-15% gain.
No one seems to have commented on that so everyone on list thinks that
aspect is OK? Any explicit ACKs?
Kernel merge windows is coming up and I would like to see whether
any of vhost-blk / vhost-scsi is going to be actually used by userspace.
I guess we could tag it for staging but would be nice to avoid that.
--
MST
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Paolo Bonzini @ 2012-07-17 10:14 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-aio, kvm, linux-kernel, virtualization, Benjamin LaHaise,
Alexander Viro, linux-fsdevel
In-Reply-To: <20120717094526.GC7949@redhat.com>
Il 17/07/2012 11:45, Michael S. Tsirkin ha scritto:
>> So it begs the question, is it going to be used in production, or just a
>> useful reference tool?
>
> Sticking to raw already makes virtio-blk faster, doesn't it?
> In that vhost-blk looks to me like just another optimization option.
> Ideally I think user just should not care where do we handle virtio:
> in-kernel or in userspace. One can imagine it being enabled/disabled
> automatically if none of the features unsupported by it are used.
Ok, that would make more sense. One difference between vhost-blk and
vhost-net is that for vhost-blk there are also management actions that
would trigger the switch, for example a live snapshot.
So a prerequisite for vhost-blk would be that it is possible to disable
it on the fly while the VM is running, as soon as all in-flight I/O is
completed.
(Note that, however, this is not possible for vhost-scsi, because it
really exposes different hardware to the guest. It must not happen that
a kernel upgrade or downgrade toggles between userspace SCSI and
vhost-scsi, for example).
>> having to
>> support the API; having to handle transition from one more thing when
>> something better comes out.
>
> Well this is true for any code. If the limited featureset which
> vhost-blk can accelerate is something many people use, then accelerating
> by 5-15% might outweight support costs.
It is definitely what people use if they are interested in performance.
Paolo
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Michael S. Tsirkin @ 2012-07-17 9:51 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-aio, kvm, linux-kernel, virtualization, Benjamin LaHaise,
Alexander Viro, linux-fsdevel
In-Reply-To: <5005313D.1000300@redhat.com>
On Tue, Jul 17, 2012 at 11:32:45AM +0200, Paolo Bonzini wrote:
> Il 17/07/2012 11:21, Asias He ha scritto:
> >> It depends. Like vhost-scsi, vhost-blk has the problem of a crippled
> >> feature set: no support for block device formats, non-raw protocols,
> >> etc. This makes it different from vhost-net.
> >
> > Data-plane qemu also has this cripppled feature set problem, no?
>
> Yes, but that is just a proof of concept. We can implement a separate
> I/O thread within the QEMU block layer, and add fast paths that resemble
> data-path QEMU, without limiting the feature set.
>
> > Does user always choose to use block devices format like qcow2? What
> > if they prefer raw image or raw block device?
>
> If they do, the code should hit fast paths and be fast. But it should
> be automatic, without the need for extra knobs. aio=thread vs.
> aio=native is already one knob too much IMHO.
Well one extra knob at qemu level is harmless IMO since
the complexity can be handled by libvirt. For vhost-net
libvirt already enables vhost automatically dependeing on backend
used and I imagine a similar thing can happen here.
> >> So it begs the question, is it going to be used in production, or just a
> >> useful reference tool?
> >
> > This should be decided by user, I can not speak for them. What is wrong
> > with adding one option for user which they can decide?
>
> Having to explain the user about the relative benefits;
This can just be done automatically by libvirt.
> having to
> support the API; having to handle transition from one more thing when
> something better comes out.
>
> Paolo
Well this is true for any code. If the limited featureset which
vhost-blk can accelerate is something many people use, then accelerating
by 5-15% might outweight support costs.
--
MST
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Michael S. Tsirkin @ 2012-07-17 9:45 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-aio, kvm, linux-kernel, virtualization, Benjamin LaHaise,
Alexander Viro, linux-fsdevel
In-Reply-To: <500527BA.9000001@redhat.com>
On Tue, Jul 17, 2012 at 10:52:10AM +0200, Paolo Bonzini wrote:
> Il 17/07/2012 10:29, Asias He ha scritto:
> > So, vhost-blk at least saves ~6 syscalls for us in each request.
>
> Are they really 6? If I/O is coalesced by a factor of 3, for example
> (i.e. each exit processes 3 requests), it's really 2 syscalls per request.
>
> Also, is there anything we can improve? Perhaps we can modify epoll and
> ask it to clear the eventfd for us (would save 2 reads)? Or
> io_getevents (would save 1)?
>
> > I guess you mean qemu here. Yes, in theory, qemu's block layer can be
> > improved to achieve similar performance as vhost-blk or kvm tool's
> > userspace virito-blk has. But I think it makes no sense to prevent one
> > solution becase there is another in theory solution called: we can do
> > similar in qemu.
>
> It depends. Like vhost-scsi, vhost-blk has the problem of a crippled
> feature set: no support for block device formats, non-raw protocols,
> etc. This makes it different from vhost-net.
Well vhost-net is also more limited than virtio-net: no support for
userspace networking, no support for level interrupts,
no support for legacy qemu vlans, can not trace datapath in userspace,
only virtio is supported. None of these is fundamental but this is
how our implementation currently behaves so from user's point of view
that's how it is. There are also fundamental limitations - e.g.
it's linux only, a special module needs to be loaded and user needs to
get an fd to the char device ... The way we addressed it, is by making it seamless for
the user: basically if your setup matches what vhost-net can
accelerate, it gets enabled, if not - userspace is used. Most of the
logic is in libvirt.
> So it begs the question, is it going to be used in production, or just a
> useful reference tool?
>
> Paolo
Sticking to raw already makes virtio-blk faster, doesn't it?
In that vhost-blk looks to me like just another optimization option.
Ideally I think user just should not care where do we handle virtio:
in-kernel or in userspace. One can imagine it being enabled/disabled
automatically if none of the features unsupported by it are used.
For example currently you specify vhost=on for tap backend and
then if you try to setup an unsupported by it like level interrupts,
it gets disabled and userspace virtio is used.
--
MST
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Paolo Bonzini @ 2012-07-17 9:32 UTC (permalink / raw)
To: Asias He
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel
In-Reply-To: <50052E7E.6020100@redhat.com>
Il 17/07/2012 11:21, Asias He ha scritto:
>> It depends. Like vhost-scsi, vhost-blk has the problem of a crippled
>> feature set: no support for block device formats, non-raw protocols,
>> etc. This makes it different from vhost-net.
>
> Data-plane qemu also has this cripppled feature set problem, no?
Yes, but that is just a proof of concept. We can implement a separate
I/O thread within the QEMU block layer, and add fast paths that resemble
data-path QEMU, without limiting the feature set.
> Does user always choose to use block devices format like qcow2? What
> if they prefer raw image or raw block device?
If they do, the code should hit fast paths and be fast. But it should
be automatic, without the need for extra knobs. aio=thread vs.
aio=native is already one knob too much IMHO.
>> So it begs the question, is it going to be used in production, or just a
>> useful reference tool?
>
> This should be decided by user, I can not speak for them. What is wrong
> with adding one option for user which they can decide?
Having to explain the user about the relative benefits; having to
support the API; having to handle transition from one more thing when
something better comes out.
Paolo
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Asias He @ 2012-07-17 9:21 UTC (permalink / raw)
To: Paolo Bonzini
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel
In-Reply-To: <500527BA.9000001@redhat.com>
On 07/17/2012 04:52 PM, Paolo Bonzini wrote:
> Il 17/07/2012 10:29, Asias He ha scritto:
>> So, vhost-blk at least saves ~6 syscalls for us in each request.
>
> Are they really 6? If I/O is coalesced by a factor of 3, for example
> (i.e. each exit processes 3 requests), it's really 2 syscalls per request.
Well. I am counting the number of syscalls in one notify and response
process. Sure the IO can be coalesced.
> Also, is there anything we can improve? Perhaps we can modify epoll and
> ask it to clear the eventfd for us (would save 2 reads)? Or
> io_getevents (would save 1)?
>
>> I guess you mean qemu here. Yes, in theory, qemu's block layer can be
>> improved to achieve similar performance as vhost-blk or kvm tool's
>> userspace virito-blk has. But I think it makes no sense to prevent one
>> solution becase there is another in theory solution called: we can do
>> similar in qemu.
>
> It depends. Like vhost-scsi, vhost-blk has the problem of a crippled
> feature set: no support for block device formats, non-raw protocols,
> etc. This makes it different from vhost-net.
Data-plane qemu also has this cripppled feature set problem, no? Does
user always choose to use block devices format like qcow2? What if they
prefer raw image or raw block device?
>
> So it begs the question, is it going to be used in production, or just a
> useful reference tool?
This should be decided by user, I can not speak for them. What is wrong
with adding one option for user which they can decide?
--
Asias
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Paolo Bonzini @ 2012-07-17 8:52 UTC (permalink / raw)
To: Asias He
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel
In-Reply-To: <50052276.2080906@redhat.com>
Il 17/07/2012 10:29, Asias He ha scritto:
> So, vhost-blk at least saves ~6 syscalls for us in each request.
Are they really 6? If I/O is coalesced by a factor of 3, for example
(i.e. each exit processes 3 requests), it's really 2 syscalls per request.
Also, is there anything we can improve? Perhaps we can modify epoll and
ask it to clear the eventfd for us (would save 2 reads)? Or
io_getevents (would save 1)?
> I guess you mean qemu here. Yes, in theory, qemu's block layer can be
> improved to achieve similar performance as vhost-blk or kvm tool's
> userspace virito-blk has. But I think it makes no sense to prevent one
> solution becase there is another in theory solution called: we can do
> similar in qemu.
It depends. Like vhost-scsi, vhost-blk has the problem of a crippled
feature set: no support for block device formats, non-raw protocols,
etc. This makes it different from vhost-net.
So it begs the question, is it going to be used in production, or just a
useful reference tool?
Paolo
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Asias He @ 2012-07-17 8:29 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel
In-Reply-To: <CAJSP0QWgmXns89se+xdGgM6i1_hsfVWPQ8caHua9d-dDA4CTDQ@mail.gmail.com>
On 07/16/2012 07:58 PM, Stefan Hajnoczi wrote:
> On Thu, Jul 12, 2012 at 4:35 PM, Asias He <asias@redhat.com> wrote:
>> This patchset adds vhost-blk support. vhost-blk is a in kernel virito-blk
>> device accelerator. Compared to userspace virtio-blk implementation, vhost-blk
>> gives about 5% to 15% performance improvement.
>
> Why is it 5-15% faster? vhost-blk and the userspace virtio-blk you
> benchmarked should be doing basically the same thing:
>
> 1. An eventfd file descriptor is signalled when the vring has new
> requests available from the guest.
> 2. A thread wakes up and processes the virtqueue.
> 3. Linux AIO is used to issue host I/O.
> 4. An interrupt is injected into the guest.
Yes. This is how both of them work. Though, there are some differences
in details. e.g.
In vhost-blk, we use the vhost's work infrastructure to handle the
requests. In kvm tool, we use a dedicated thread.
In vhost-blk, we use irqfd to inject interrupts. In kvm tool, we use
ioctl to inject interrupts.
> Does the vhost-blk implementation do anything fundamentally different
> from userspace? Where is the overhead that userspace virtio-blk has?
Currently, no. But we could play with bio directly in vhost-blk as
Christoph suggested which could make the IO path from guest to host's
real storage even shorter in vhost-blk.
I've been trying my best to reduce the overhead of virtio-blk at kvm
tool side. I do not see any significant overhead out there. Compared to
vhost-blk, the overhead we have in userspace virito-blk is syscalls. In
each IO request, we have
epoll_wait() & read(): wait for the eventfd which guest notifies us
io_submit(): submit the aio
read(): read the aio complete eventfd
io_getevents(): reap the aio complete result
ioctl(): trigger the interrupt
So, vhost-blk at least saves ~6 syscalls for us in each request.
> I'm asking because it would be beneficial to fix the overhead
> (especially it that could speed up all userspace applications) instead
> of adding a special-purpose kernel module to work around the overhead.
I guess you mean qemu here. Yes, in theory, qemu's block layer can be
improved to achieve similar performance as vhost-blk or kvm tool's
userspace virito-blk has. But I think it makes no sense to prevent one
solution becase there is another in theory solution called: we can do
similar in qemu.
What do you mean by specail-purpose here, we need general-purpose kernel
module? Is vhost-net a special purpose kernel module? Is xen-blkback a
special-purpose kernel module? And I think vhost-blk is beneficial to
qemu too, as well as any other kvm host side implementation.
--
Asias
^ permalink raw reply
* Re: [PATCH 2/2] virtio-scsi spec: add configuration change event
From: Rusty Russell @ 2012-07-17 0:21 UTC (permalink / raw)
To: Paolo Bonzini, kvm, virtualization; +Cc: qemu-devel
In-Reply-To: <1342448677-7453-3-git-send-email-pbonzini@redhat.com>
On Mon, 16 Jul 2012 16:24:37 +0200, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This adds an event for changes to LUN parameters, for example capacity. These
> are reported in virtio-blk via configuration changes, and we want a similar
> functionality in virtio-scsi too.
Both applied.
Thanks!
Rusty.
^ permalink raw reply
* [PATCH 2/2] virtio-scsi spec: add configuration change event
From: Paolo Bonzini @ 2012-07-16 14:24 UTC (permalink / raw)
To: kvm, virtualization, rusty; +Cc: qemu-devel
In-Reply-To: <1342448677-7453-1-git-send-email-pbonzini@redhat.com>
This adds an event for changes to LUN parameters, for example capacity. These
are reported in virtio-blk via configuration changes, and we want a similar
functionality in virtio-scsi too.
There is no list of supported parameter changes, instead we just refer to
the list of sense codes in the SCSI specification.
This event will usually be serviced in one of three ways: 1) call an OS
service to revalidate the disk, either always or only for some specific
sense codes; 2) somehow pass the sense directly to the upper-level driver;
3) inject a TEST UNIT READY command into the upper-level device, so that
the OS will see the unit attention code and react. Of course a mix of
the three is also possible, depending on how the driver writer prefers
to have his layering violations served.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
virtio-spec.lyx | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index f8b214b..8d2ac9a 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -6995,6 +6995,21 @@ VIRTIO_SCSI_F_HOTPLUG
(1) The host should enable hot-plug/hot-unplug of new LUNs and targets on
the SCSI bus.
+\change_inserted 1531152142 1342440342
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 1531152142 1342440768
+VIRTIO_SCSI_F_CHANGE
+\begin_inset space ~
+\end_inset
+
+(2) The host will report changes to LUN parameters via a VIRTIO_SCSI_T_PARAM_CHA
+NGE event.
+\change_unchanged
+
\end_layout
\end_deeper
@@ -8673,6 +8688,86 @@ reason
\begin_layout Standard
When dropped events are reported, the driver should poll for asynchronous
events manually using SCSI commands.
+\change_inserted 1531152142 1342439104
+
+\end_layout
+
+\end_deeper
+\begin_layout Description
+
+\change_inserted 1531152142 1342440778
+LUN
+\begin_inset space ~
+\end_inset
+
+parameter
+\begin_inset space ~
+\end_inset
+
+change
+\begin_inset space ~
+\end_inset
+
+
+\begin_inset Newline newline
+\end_inset
+
+
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 1531152142 1342440783
+
+#define VIRTIO_SCSI_T_PARAM_CHANGE 3
+\end_layout
+
+\end_inset
+
+
+\end_layout
+
+\begin_deeper
+\begin_layout Standard
+
+\change_inserted 1531152142 1342440882
+By sending this event, the device signals that the configuration parameters
+ (for example the capacity) of a logical unit have changed.
+ The
+\series bold
+event
+\series default
+ field is set to VIRTIO_SCSI_T_PARAM_CHANGE.
+ The
+\series bold
+lun
+\series default
+ field addresses a logical unit in the SCSI host.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 1531152142 1342440916
+The same event is also reported as a unit attention condition.
+ The
+\series bold
+reason
+\series default
+ field contains the additional sense code and additional sense code qualifier,
+ respectively in bits 0..7 and 8..15.
+ For example, a change in capacity will be reported as asc 0x2a, ascq 0x09
+ (CAPACITY DATA HAS CHANGED).
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 1531152142 1342442803
+For MMC devices (inquiry type 5) there would be some overlap between this
+ event and the asynchronous notification event.
+ For simplicity, as of this version of the specification the host must
+ never report this event for MMC devices.
\end_layout
\end_deeper
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/2] virtio-scsi spec: unify event structs
From: Paolo Bonzini @ 2012-07-16 14:24 UTC (permalink / raw)
To: kvm, virtualization, rusty; +Cc: qemu-devel
In-Reply-To: <1342448677-7453-1-git-send-email-pbonzini@redhat.com>
All currently defined event structs have the same fields. Simplify the
driver by enforcing this also for future structs.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
virtio-spec.lyx | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 905e619..f8b214b 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -8207,7 +8207,20 @@ struct virtio_scsi_event {
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440791
+
...
+\change_inserted 1531152142 1342440791
+u8 lun[8];
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 1531152142 1342440791
+
+ u32 reason;
+\change_unchanged
+
\end_layout
\begin_layout Plain Layout
@@ -8221,16 +8234,32 @@ struct virtio_scsi_event {
\end_layout
\begin_layout Standard
-If bit 31 is set in the event field, the device failed to report an event
- due to missing buffers.
+If bit 31 is set in the
+\series bold
+event
+\series default
+ field, the device failed to report an event due to missing buffers.
In this case, the driver should poll the logical units for unit attention
conditions, and/or do whatever form of bus scan is appropriate for the
guest operating system.
\end_layout
\begin_layout Standard
-Other data that the device writes to the buffer depends on the contents
- of the event field.
+
+\change_deleted 1531152142 1342440830
+Other data that the device writes to the buffer
+\change_inserted 1531152142 1342440839
+The meaning of the
+\series bold
+reason
+\series default
+ field
+\change_unchanged
+ depends on the contents of the
+\series bold
+event
+\series default
+ field.
The following events are defined:
\end_layout
@@ -8312,36 +8341,50 @@ status open
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440799
+
struct virtio_scsi_event_reset {
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440799
+
// Write-only part
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440799
+
u32 event;
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440799
+
u8 lun[8];
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440799
+
u32 reason;
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440799
+
}
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440799
+
\end_layout
\begin_layout Plain Layout
@@ -8542,40 +8585,58 @@ status open
\begin_layout Plain Layout
#define VIRTIO_SCSI_T_ASYNC_NOTIFY 2
+\change_deleted 1531152142 1342440854
+
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440854
+
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440854
+
struct virtio_scsi_event_an {
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440854
+
// Write-only part
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440854
+
u32 event;
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440854
+
u8 lun[8];
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440854
+
u32 reason;
\end_layout
\begin_layout Plain Layout
+\change_deleted 1531152142 1342440854
+
}
+\change_unchanged
+
\end_layout
\end_inset
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/2] virtio-scsi spec: event improvements
From: Paolo Bonzini @ 2012-07-16 14:24 UTC (permalink / raw)
To: kvm, virtualization, rusty; +Cc: qemu-devel
This makes some changes to the virtio-scsi event specification, so that
it is now possible to use virtio-scsi events in the implementation of
the QEMU "block_resize" command.
Thanks to Cong Meng for finally implementing virtio-scsi hotplug, which
made me look at block_resize again!
Paolo Bonzini (2):
virtio-scsi spec: unify event structs
virtio-scsi spec: add configuration change event
virtio-spec.lyx | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 160 insertions(+), 4 deletions(-)
--
1.7.10.4
^ permalink raw reply
* Re: [PATCH 0/5] Add vhost-blk support
From: Stefan Hajnoczi @ 2012-07-16 11:58 UTC (permalink / raw)
To: Asias He
Cc: linux-aio, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Benjamin LaHaise, Alexander Viro, linux-fsdevel
In-Reply-To: <1342107302-28116-1-git-send-email-asias@redhat.com>
On Thu, Jul 12, 2012 at 4:35 PM, Asias He <asias@redhat.com> wrote:
> This patchset adds vhost-blk support. vhost-blk is a in kernel virito-blk
> device accelerator. Compared to userspace virtio-blk implementation, vhost-blk
> gives about 5% to 15% performance improvement.
Why is it 5-15% faster? vhost-blk and the userspace virtio-blk you
benchmarked should be doing basically the same thing:
1. An eventfd file descriptor is signalled when the vring has new
requests available from the guest.
2. A thread wakes up and processes the virtqueue.
3. Linux AIO is used to issue host I/O.
4. An interrupt is injected into the guest.
Does the vhost-blk implementation do anything fundamentally different
from userspace? Where is the overhead that userspace virtio-blk has?
I'm asking because it would be beneficial to fix the overhead
(especially it that could speed up all userspace applications) instead
of adding a special-purpose kernel module to work around the overhead.
Stefan
^ permalink raw reply
* Re: [PATCH RESEND 0/5] Add vhost-blk support
From: Asias He @ 2012-07-16 9:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-aio, Dave Kleikamp, Zach Brown, kvm, Michael S. Tsirkin,
linux-kernel, virtualization, James Bottomley, Jeff Moyer,
Benjamin LaHaise, Alexander Viro, linux-fsdevel
In-Reply-To: <20120714074911.GA31684@infradead.org>
Hi Christoph,
On 07/14/2012 03:49 PM, Christoph Hellwig wrote:
> Please send a version that does direct block I/O similar to xen-blkback
> for now.
Seems xen-blkback converts the guest IO request to host bio and submit
them directly. I was wondering whether this has a performance gain
compared to AIO implementation.
> If we get proper in-kernel aio support one day you can add
> back file backend support.
I talked with Dave and Zack on the in-kernel aio patch which James
pointed out:
http://marc.info/?l=linux-fsdevel&m=133312234313122
Dave will post a new version soon. I will wait for it.
--
Asias
^ permalink raw reply
* Re: [PATCH] hw/virtio-scsi: Set max_target=0 during vhost-scsi operation
From: Zhi Yong Wu @ 2012-07-16 1:55 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin, qemu-devel,
lf-virt, target-devel, Paolo Bonzini, Zhi Yong Wu,
linux-iscsi-target-dev
In-Reply-To: <1342130918.18004.116.camel@haakon2.linux-iscsi.org>
HI, nab,
On Fri, Jul 13, 2012 at 6:08 AM, Nicholas A. Bellinger
<nab@linux-iscsi.org> wrote:
> Hi Zhi,
>
> On Thu, 2012-07-12 at 14:59 +0800, Zhi Yong Wu wrote:
>> thanks, it is applied to my vhost_scsi git tree
>> git://github.com/wuzhy/qemu.git vhost-scsi
>>
>
> Thanks for picking up this patch in your vhost-scsi tree.
>
> As mentioned off-list, I'd like to rebase to a more recent qemu.git to
> include megasas 8708EM2 HBA emulation from Dr. Hannes so we can
> experiment with a few more types of target setups. ;)
I have rebased my vhost-scsi tree to latest qemu.git.
>
> I'll likely do this on my local branch for now, but if you have the
> extra cycles please feel free to update vhost-scsi to the latest
> qemu.git HEAD so we can have both vhost-scsi + megasas HBA emulation in
> the same working tree.
I have push rebased vhost-scsi tree to my public git.
>
> Depending upon how long we'll need to hold vhost-scsi patches
> out-of-tree (hopefully it's less than infinity ;) a qemu/vhost-scsi
Heh, i also hope so, but have not anthority to push it to kernel.org.
> working tree on kernel.org might also be helpful.
>
> Thanks!
>
> --nab
>
>
>
--
Regards,
Zhi Yong Wu
^ permalink raw reply
* RE: [PATCH 01/15] Drivers: hv: Format GUIDS as per MSFT standards
From: Joe Perches @ 2012-07-14 23:28 UTC (permalink / raw)
To: KY Srinivasan
Cc: olaf@aepfle.de, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org,
apw@canonical.com, devel@linuxdriverproject.org
In-Reply-To: <426367E2313C2449837CD2DE46E7EAF9223B702C@SN2PRD0310MB382.namprd03.prod.outlook.com>
On Sat, 2012-07-14 at 23:23 +0000, KY Srinivasan wrote:
> > -----Original Message-----
> > From: Joe Perches [mailto:joe@perches.com]
> > Sent: Saturday, July 14, 2012 4:25 PM
[]
> > On Sat, 2012-07-14 at 13:34 -0700, K. Y. Srinivasan wrote:
> > > Format GUIDS as per MSFT standard. This makes interacting with MSFT
> > > tool stack easier.
[]
> > > @@ -166,7 +166,7 @@ static ssize_t vmbus_show_device_attr(struct device
> > *dev,
> > > device_info->chn_type.b[15]);
> > > } else if (!strcmp(dev_attr->attr.name, "device_id")) {
> > > ret = sprintf(buf, "{%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > > - "%02x%02x%02x%02x%02x%02x%02x%02x}\n",
> > > + "%02x%02x-%02x%02x%02x%02x%02x%02x}\n",
> > > device_info->chn_instance.b[3],
> > > device_info->chn_instance.b[2],
> > > device_info->chn_instance.b[1],
> >
> > ret = sprintf(buf, "{%pUl}\n", device_info->chn_instance.b);
>
> Thank you Joe. I recall seeing some patches from you a longtime ago on this.
> I was just modifying existing code in this patch; if it is ok with you I will send a
> separate patch using the preferred format string for printing GUIDS.
Hi KY. It's your code. Do what you think best. cheers, Joe
^ permalink raw reply
* RE: [PATCH 01/15] Drivers: hv: Format GUIDS as per MSFT standards
From: KY Srinivasan @ 2012-07-14 23:23 UTC (permalink / raw)
To: Joe Perches
Cc: olaf@aepfle.de, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org,
apw@canonical.com, devel@linuxdriverproject.org
In-Reply-To: <1342297484.8377.28.camel@joe2Laptop>
> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Saturday, July 14, 2012 4:25 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com
> Subject: Re: [PATCH 01/15] Drivers: hv: Format GUIDS as per MSFT standards
>
> On Sat, 2012-07-14 at 13:34 -0700, K. Y. Srinivasan wrote:
> > Format GUIDS as per MSFT standard. This makes interacting with MSFT
> > tool stack easier.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > drivers/hv/vmbus_drv.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index a220e57..1f7e54a 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -147,7 +147,7 @@ static ssize_t vmbus_show_device_attr(struct device
> *dev,
> >
> > if (!strcmp(dev_attr->attr.name, "class_id")) {
> > ret = sprintf(buf, "{%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > - "%02x%02x%02x%02x%02x%02x%02x%02x}\n",
> > + "%02x%02x-%02x%02x%02x%02x%02x%02x}\n",
> > device_info->chn_type.b[3],
> > device_info->chn_type.b[2],
> > device_info->chn_type.b[1],
> > @@ -166,7 +166,7 @@ static ssize_t vmbus_show_device_attr(struct device
> *dev,
> > device_info->chn_type.b[15]);
> > } else if (!strcmp(dev_attr->attr.name, "device_id")) {
> > ret = sprintf(buf, "{%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > - "%02x%02x%02x%02x%02x%02x%02x%02x}\n",
> > + "%02x%02x-%02x%02x%02x%02x%02x%02x}\n",
> > device_info->chn_instance.b[3],
> > device_info->chn_instance.b[2],
> > device_info->chn_instance.b[1],
>
> ret = sprintf(buf, "{%pUl}\n", device_info->chn_instance.b);
Thank you Joe. I recall seeing some patches from you a longtime ago on this.
I was just modifying existing code in this patch; if it is ok with you I will send a
separate patch using the preferred format string for printing GUIDS.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 12/15] Tools: hv: Gather DHCP information
From: richard -rw- weinberger @ 2012-07-14 23:04 UTC (permalink / raw)
To: KY Srinivasan
Cc: olaf@aepfle.de, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org,
apw@canonical.com, devel@linuxdriverproject.org
In-Reply-To: <426367E2313C2449837CD2DE46E7EAF9223B7004@SN2PRD0310MB382.namprd03.prod.outlook.com>
On Sun, Jul 15, 2012 at 1:02 AM, KY Srinivasan <kys@microsoft.com> wrote:
> I have a bunch of other cleanups lined up for this user level daemon. If it is
> ok with you, I could address this as part of that patch set. However, if
> there are more substantive changes needed in this patch-set, I will
> also address this.
No problem.
--
Thanks,
//richard
^ permalink raw reply
* RE: [PATCH 12/15] Tools: hv: Gather DHCP information
From: KY Srinivasan @ 2012-07-14 23:02 UTC (permalink / raw)
To: richard -rw- weinberger
Cc: olaf@aepfle.de, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org,
apw@canonical.com, devel@linuxdriverproject.org
In-Reply-To: <CAFLxGvxkidfN+o8yFsmGm8-YhT4Q5dgzQOAJJXnM=nXoXVpqTA@mail.gmail.com>
> -----Original Message-----
> From: richard -rw- weinberger [mailto:richard.weinberger@gmail.com]
> Sent: Saturday, July 14, 2012 5:37 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com
> Subject: Re: [PATCH 12/15] Tools: hv: Gather DHCP information
>
> On Sat, Jul 14, 2012 at 10:34 PM, K. Y. Srinivasan <kys@microsoft.com> wrote:
> > Collect information on dhcp setting for the specified interface.
> > We invoke an exyernal script to get this information.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > tools/hv/hv_kvp_daemon.c | 33 +++++++++++++++++++++++++++++++++
> > 1 files changed, 33 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> > index a81ce67..c510283 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -524,6 +524,9 @@ static void kvp_get_ipconfig_info(char *if_name,
> > struct hv_kvp_ipaddr_value *buffer)
> > {
> > char cmd[512];
> > + char dhcp_info[128];
> > + char *p;
> > + FILE *file;
> >
> > /*
> > * Get the address of default gateway (ipv4).
> > @@ -580,6 +583,36 @@ static void kvp_get_ipconfig_info(char *if_name,
> > */
> > kvp_process_ipconfig_file(cmd, (char *)buffer->dns_addr,
> > (MAX_IP_ADDR_SIZE * 2), INET_ADDRSTRLEN, 0);
> > +
> > + /*
> > + * Gather the DHCP state.
> > + * We will gather this state by invoking an external script.
> > + * The parameter to the script is the interface name.
> > + * Here is the expected output:
> > + *
> > + * Enabled: DHCP enabled.
> > + */
> > +
> > + memset(cmd, 0, 512);
>
> Why not sizeof(cmd)?
I have a bunch of other cleanups lined up for this user level daemon. If it is
ok with you, I could address this as part of that patch set. However, if
there are more substantive changes needed in this patch-set, I will
also address this.
Thank you,
K. Y
^ permalink raw reply
* Re: [patch -next] tcm_vhost: another strlen() off by one
From: Nicholas A. Bellinger @ 2012-07-14 22:21 UTC (permalink / raw)
To: Dan Carpenter
Cc: target-devel, virtualization, kernel-janitors, kvm,
Michael S. Tsirkin
In-Reply-To: <20120713104547.GA31435@elgon.mountain>
On Fri, 2012-07-13 at 13:45 +0300, Dan Carpenter wrote:
> strlen() doesn't count the NUL terminator. I missed this one in the
> patches I sent yesterday.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> index 29850cb..ea72198 100644
> --- a/drivers/vhost/tcm_vhost.c
> +++ b/drivers/vhost/tcm_vhost.c
> @@ -1424,7 +1424,7 @@ static struct se_wwn *tcm_vhost_make_tport(
> return ERR_PTR(-EINVAL);
>
> check_len:
> - if (strlen(name) > TCM_VHOST_NAMELEN) {
> + if (strlen(name) >= TCM_VHOST_NAMELEN) {
> pr_err("Emulated %s Address: %s, exceeds"
> " max: %d\n", name, tcm_vhost_dump_proto_id(tport),
> TCM_VHOST_NAMELEN);
Applied + squashed into the initial merge commit in for-next-merge.
Thanks DanC!
--nab
^ permalink raw reply
* Re: [PATCH 12/15] Tools: hv: Gather DHCP information
From: richard -rw- weinberger @ 2012-07-14 21:36 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, virtualization, olaf, apw
In-Reply-To: <1342298060-12662-12-git-send-email-kys@microsoft.com>
On Sat, Jul 14, 2012 at 10:34 PM, K. Y. Srinivasan <kys@microsoft.com> wrote:
> Collect information on dhcp setting for the specified interface.
> We invoke an exyernal script to get this information.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> tools/hv/hv_kvp_daemon.c | 33 +++++++++++++++++++++++++++++++++
> 1 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index a81ce67..c510283 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -524,6 +524,9 @@ static void kvp_get_ipconfig_info(char *if_name,
> struct hv_kvp_ipaddr_value *buffer)
> {
> char cmd[512];
> + char dhcp_info[128];
> + char *p;
> + FILE *file;
>
> /*
> * Get the address of default gateway (ipv4).
> @@ -580,6 +583,36 @@ static void kvp_get_ipconfig_info(char *if_name,
> */
> kvp_process_ipconfig_file(cmd, (char *)buffer->dns_addr,
> (MAX_IP_ADDR_SIZE * 2), INET_ADDRSTRLEN, 0);
> +
> + /*
> + * Gather the DHCP state.
> + * We will gather this state by invoking an external script.
> + * The parameter to the script is the interface name.
> + * Here is the expected output:
> + *
> + * Enabled: DHCP enabled.
> + */
> +
> + memset(cmd, 0, 512);
Why not sizeof(cmd)?
> + strcat(cmd, "/sbin/hv_get_dhcp_info ");
> + strcat(cmd, if_name);
What about strncat()?
--
Thanks,
//richard
^ 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