* Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
From: Olaf Hering @ 2012-07-25 7:59 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, apw, netdev, ben
In-Reply-To: <1343145701-3691-3-git-send-email-kys@microsoft.com>
On Tue, Jul 24, K. Y. Srinivasan wrote:
> +++ b/drivers/hv/hv_kvp.c
> @@ -48,13 +48,24 @@ static struct {
> void *kvp_context; /* for the channel callback */
> } kvp_transaction;
>
> +/*
> + * Before we can accept KVP messages from the host, we need
> + * to handshake with the user level daemon. This state tarcks
tracks
> + * if we are in the handshake phase.
> + */
> - * Something failed or the we have timedout;
> + * Something failed or we have timedout;
extra space
^ permalink raw reply
* Re: Re: [Qemu-devel] [RFC PATCH 0/6] virtio-trace: Support virtio-trace
From: Masami Hiramatsu @ 2012-07-25 8:15 UTC (permalink / raw)
To: Blue Swirl
Cc: Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman,
Frederic Weisbecker, linux-kernel, Borislav Petkov, qemu-devel,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, yrl.pp-manager.tt, Amit Shah, virtualization
In-Reply-To: <CAAu8pHsfRBsfSCB5gBFX5pweN2j2PdNrQZyCfjJ3sYYmVjCRfA@mail.gmail.com>
(2012/07/25 5:26), Blue Swirl wrote:>
>> The following patch set provides a low-overhead system for collecting kernel
>> tracing data of guests by a host in a virtualization environment.
>>
>> A guest OS generally shares some devices with other guests or a host, so
>> reasons of any problems occurring in a guest may be from other guests or a
>> host.
>> Then, to collect some tracing data of a number of guests and a host is needed
>> when some problems occur in a virtualization environment. One of methods to
>> realize that is to collect tracing data of guests in a host. To do this,
>> network
>> is generally used. However, high load will be taken to applications on guests
>> using network I/O because there are many network stack layers. Therefore,
>> a communication method for collecting the data without using network is
>> needed.
>
> I implemented something similar earlier by passing trace data from
> OpenBIOS to QEMU using the firmware configuration device. The data
> format was the same as QEMU used for simpletrace event structure
> instead of ftrace. I didn't commit it because of a few problems.
Sounds interesting :)
I guess you traced BIOS events, right?
> I'm not familiar with ftrace, is it possible to trace two guest
> applications (BIOS and kernel) at the same time?
Since ftrace itself is a tracing feature in the linux kernel, it
can trace two or more applications (processes) if those run on linux
kernel. However, I think OpenBIOS runs *under* the guest kernel.
If so, ftrace currently can't trace OpenBIOS from guest side.
I think it may need another enhancement on both OpenBIOS and linux
kernel to trace BIOS event from linux kernel.
> Or could this be
> handled by opening two different virtio-serial pipes, one for BIOS and
> the other for the kernel?
Of course, virtio-serial itself can open multiple channels, thus, if
OpenBIOS can handle virtio, it can pass trace data via another
channel.
> In my version, the tracepoint ID would have been used to demultiplex
> QEMU tracepoints from BIOS tracepoints, but something like separate ID
> spaces would have been better.
I guess your feature notifies events to QEMU and QEMU records that in
their own buffer. Therefore it must have different tracepoint IDs.
On the other hand, with this feature, QEMU just passes trace-data to
host-side pipe. Since outer tracing tool separately collects trace
data, we don't need to demultiplex the data.
Perhaps, in the analyzing phase (after tracing), we have to mix events
again. At that time, we'll add some guest-ID for each event-ID, but
it can be done offline.
Best Regards,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply
* Re: [RFC 0/2] virtio: provide a way for host to monitor critical events in the device
From: Amit Shah @ 2012-07-25 8:46 UTC (permalink / raw)
To: Rusty Russell
Cc: Peter Maydell, kvm, wency, mst, linux-kernel, virtualization,
penberg, Sasha Levin, anthony, avi
In-Reply-To: <87haswptgq.fsf@rustcorp.com.au>
On (Wed) 25 Jul 2012 [10:06:37], Rusty Russell wrote:
> On Tue, 24 Jul 2012 15:01:59 +0200, Sasha Levin <levinsasha928@gmail.com> wrote:
> > virtio on it's own was introduced to help solve the fragmentation
> > around virtualized devices, so I don't think that the main purpose of
> > doing virtio drivers is due to any performance benefits virtio may
> > provide.
>
> There's one argument in your favor (with my Linaro hat on): ARM wants a
> virtio reboot button, which would look remarkably similar. There's no
> standard ARM hardware for this.
>
> So a more generalized virtio-event device might make sense. But there
> are almost an infinite number of guest events we might want: panics,
> oom, low memory, stuck devices, deadlock, etc, etc. I'm concerned about
> trying to standardize them. If we include a unspecified free-form
> string, people will end up relying on the contents. If we add a feature
> bit for every new event, we'll end up running out of feature bits :)
>
> CC'ing Amit for opinion over how much of this should be done via
> virtio-serial.
The prevoius discussion happend on kvm-devel; it was suggested then to
use virtio-serial for that as well. We don't have an in-kernel
interface for communication yet (barring the console interface, which
we don't want to re-use for other reasons).
Writing the in-kernel interface for communication with the host is not
too much work as well.
I agree using virtio-serial for several such free-form message-passing
between the guest and host is the right way to implement such stuff.
The lack of dedicated devices over either virtio or emulation of
real hardware can be overcome by adding some documentation -
preferably to the virtio spec's appendix, showing how watchdogs, etc.,
are implemented using virtio-serial.
Amit
^ permalink raw reply
* Re: Re: Re: [RFC PATCH 0/6] virtio-trace: Support virtio-trace
From: Yoshihiro YUNOMAE @ 2012-07-25 9:13 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman,
Frederic Weisbecker, Dhaval Giani, linux-kernel, Borislav Petkov,
qemu-devel, Amit Shah, Franch Ch. Eigler, Ingo Molnar,
Mathieu Desnoyers, Steven Rostedt, Anthony Liguori,
yrl.pp-manager.tt, Masami Hiramatsu, virtualization,
Srikar Dronamraju
In-Reply-To: <CAJSP0QVuQnE68n=jyW+AE=WF3yDHjgdn7tdiz7rPTbtqGYFL4w@mail.gmail.com>
Hi Stefan,
(2012/07/24 22:41), Stefan Hajnoczi wrote:
> On Tue, Jul 24, 2012 at 12:19 PM, Yoshihiro YUNOMAE
> <yoshihiro.yunomae.ez@hitachi.com> wrote:
>>>> Are you using text formatted ftrace?
>> No, currently using raw format, but we'd like to reformat it in text.
>
> Capturing the info necessary to translate numbers into symbols is one
> of the problems of host<->guest tracing so I'm curious how you handle
> this :).
Right, your consideration is true.
> Apologies for my lack of ftrace knowledge but how useful is the raw
> tracing data on the host? How do you pretty-print it in
> human-readable form?
perf and trace-cmd can actually translate raw-formatted trace data to
text-formatted trace data by using information of kernel or trace
format under tracing/events directory in debugfs. In the same way, if
the information of a guest is exported to a host, we can translate
raw trace data of a guest to text trace data on a host. We will use
9pfs to export that.
Thank you,
--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@hitachi.com
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Stefan Hajnoczi @ 2012-07-25 11:55 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, linux-scsi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, Paolo Bonzini, Zhi Yong Wu, Christoph Hellwig,
Avi Kivity
In-Reply-To: <1343162724.1813.111.camel@haakon2.linux-iscsi.org>
On Tue, Jul 24, 2012 at 01:45:24PM -0700, Nicholas A. Bellinger wrote:
> On Mon, 2012-07-23 at 18:56 -0700, Greg Kroah-Hartman wrote:
> > On Tue, Jul 24, 2012 at 01:26:20AM +0000, Nicholas A. Bellinger wrote:
> > > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > >
> > > As requested by Anthony, here is a patch against target-pending/for-next-merge
> > > to expose an ABI version to userspace via a new VHOST_SCSI_GET_ABI_VERSION
> > > ioctl operation.
> > >
> > > As mentioned in the comment, ABI Rev 0 is for pre 2012 out-of-tree code, and
> > > ABI Rev 1 (the current rev) is for current WIP v3.6 kernel merge candiate code.
> > >
> > > I think this is what you had in mind, and hopefully it will make MST happy too.
> > > The incremental vhost-scsi patches against Zhi's QEMU are going out shortly ahead
> > > of cutting a new vhost-scsi RFC over the next days.
> > >
> > > Please have a look and let me know if you have any concerns here.
> > >
> > > Thanks!
> > >
> > > Reported-by: Anthony Liguori <aliguori@us.ibm.com>
> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > > ---
> > > drivers/vhost/tcm_vhost.c | 9 +++++++++
> > > drivers/vhost/tcm_vhost.h | 11 +++++++++++
> > > 2 files changed, 20 insertions(+), 0 deletions(-)
> > >
>
> <SNIP>
>
> > > diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
> > > index e942df9..3d5378f 100644
> > > --- a/drivers/vhost/tcm_vhost.h
> > > +++ b/drivers/vhost/tcm_vhost.h
> > > @@ -80,7 +80,17 @@ struct tcm_vhost_tport {
> > >
> > > #include <linux/vhost.h>
> > >
> > > +/*
> > > + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> > > + *
> > > + * ABI Rev 0: All pre 2012 revisions used by prototype out-of-tree code
> > > + * ABI Rev 1: 2012 version for v3.6 kernel merge candiate
> > > + */
> > > +
> > > +#define VHOST_SCSI_ABI_VERSION 1
> > > +
> > > struct vhost_scsi_target {
> > > + int abi_version;
> > > unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
> > > unsigned short vhost_tpgt;
> > > };
> > > @@ -88,3 +98,4 @@ struct vhost_scsi_target {
> > > /* VHOST_SCSI specific defines */
> > > #define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> > > #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> > > +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> >
> > No, you just broke the ABI for version "0" here, that's not how you do
> > this at all.
> >
>
> The intention of this patch is use ABI=1 as a starting point for
> tcm_vhost moving forward, with no back-wards compat for the ABI=0
> prototype userspace code because:
>
> - It's based on a slightly older version of QEMU (updating the QEMU series now)
> - It does not have an GET_ABI_VERSION ioctl cmd (that starts with ABI=1)
> - It has a small user-base of target + virtio-scsi developers
>
> So I did consider just starting from ABI=0, but figured this would help
> reduce the confusion for QEMU userspace wrt to the vhost-scsi code
> that's been floating around out-of-tree for the last 2 years.
There is no real user base beyond the handful of people who have hacked
on this. Adding the GET_ABI_VERSION ioctl() at this stage is fine,
especially considering that the userspace code that talks to tcm_vhost
isn't in mainline in userspace yet either.
Stefan
^ permalink raw reply
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Avi Kivity @ 2012-07-25 13:10 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, linux-scsi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, Paolo Bonzini, Zhi Yong Wu, Christoph Hellwig,
Stefan Hajnoczi
In-Reply-To: <1343162724.1813.111.camel@haakon2.linux-iscsi.org>
On 07/24/2012 11:45 PM, Nicholas A. Bellinger wrote:
>> > diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
>> > index e942df9..3d5378f 100644
>> > --- a/drivers/vhost/tcm_vhost.h
>> > +++ b/drivers/vhost/tcm_vhost.h
>> > @@ -80,7 +80,17 @@ struct tcm_vhost_tport {
>> >
>> > #include <linux/vhost.h>
>> >
>> > +/*
>> > + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
>> > + *
>> > + * ABI Rev 0: All pre 2012 revisions used by prototype out-of-tree code
>> > + * ABI Rev 1: 2012 version for v3.6 kernel merge candiate
>> > + */
If it's out of tree, why consider it at all? Put a stable ABI in tree
and extend it in compatible ways.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* RE: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
From: KY Srinivasan @ 2012-07-25 14:10 UTC (permalink / raw)
To: Ben Hutchings
Cc: olaf@aepfle.de, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org,
netdev@vger.kernel.org, apw@canonical.com,
devel@linuxdriverproject.org
In-Reply-To: <1343178644.5132.103.camel@deadeye.wl.decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, July 24, 2012 9:11 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; netdev@vger.kernel.org
> Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
>
> On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > In preparation to implementing IP injection, cleanup the way we propagate
> > and handle errors both in the driver as well as in the user level daemon.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > drivers/hv/hv_kvp.c | 112 +++++++++++++++++++++++++++++++++++++-
> --------
> > include/linux/hyperv.h | 17 +++++---
> > tools/hv/hv_kvp_daemon.c | 70 +++++++++++++++-------------
> > 3 files changed, 138 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> > index 0012eed..9b7fc4a 100644
> > --- a/drivers/hv/hv_kvp.c
> > +++ b/drivers/hv/hv_kvp.c
> [...]
> > @@ -109,27 +154,52 @@ kvp_cn_callback(struct cn_msg *msg, struct
> netlink_skb_parms *nsp)
> > {
> > struct hv_kvp_msg *message;
> > struct hv_kvp_msg_enumerate *data;
> > + int error = 0;
> >
> > message = (struct hv_kvp_msg *)msg->data;
> > - switch (message->kvp_hdr.operation) {
> > +
> > + /*
> > + * If we are negotiating the version information
> > + * with the daemon; handle that first.
> > + */
> > +
> > + if (in_hand_shake) {
> > + if (kvp_handle_handshake(message))
> > + in_hand_shake = false;
> > + return;
> > + }
> > +
> > + /*
> > + * Based on the version of the daemon, we propagate errors from the
> > + * daemon differently.
> > + */
> > +
> > + data = &message->body.kvp_enum_data;
> > +
> > + switch (dm_reg_value) {
> > case KVP_OP_REGISTER:
> > - pr_info("KVP: user-mode registering done.\n");
> > - kvp_register();
> > - kvp_transaction.active = false;
> > - hv_kvp_onchannelcallback(kvp_transaction.kvp_context);
> > + /*
> > + * Null string is used to pass back error condition.
> > + */
> > + if (!strlen(data->data.key))
>
> Do we know that the key is null-terminated here? Shouldn't we just
> check whether data->data.key[0] == 0?
Yes, currently we do return null string to indicate error.
>
> > + error = HV_S_CONT;
> > break;
> >
> > - default:
> > - data = &message->body.kvp_enum_data;
> > + case KVP_OP_REGISTER1:
> > /*
> > - * Complete the transaction by forwarding the key value
> > - * to the host. But first, cancel the timeout.
> > + * We use the message header information from
> > + * the user level daemon to transmit errors.
> > */
> > - if (cancel_delayed_work_sync(&kvp_work))
> > - kvp_respond_to_host(data->data.key,
> > - data->data.value,
> > - !strlen(data->data.key));
> > + error = *((int *)(&message->kvp_hdr.operation));
> [...]
>
> What's with the casting (repeated in many other places)? Wouldn't it be
> better to redefine struct hv_kvp_msg to start with something like:
>
> union {
> struct hv_kvp_hdr request;
> int error;
> } kvp_hdr;
Agreed; will do.
>
> Ben.
>
> --
> Ben Hutchings
> If more than one person is responsible for a bug, no one is at fault.
^ permalink raw reply
* RE: [PATCH 08/17] Tools: hv: Gather subnet information
From: KY Srinivasan @ 2012-07-25 14:10 UTC (permalink / raw)
To: Ben Hutchings
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
olaf@aepfle.de, apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <1343178850.5132.104.camel@deadeye.wl.decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, July 24, 2012 9:14 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; netdev@vger.kernel.org
> Subject: Re: [PATCH 08/17] Tools: hv: Gather subnet information
>
> On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > Now gather sub-net information for the specified interface.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > tools/hv/hv_kvp_daemon.c | 31 +++++++++++++++++++++++++++++--
> > 1 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> > index 79eb130..2c24ebf 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -534,6 +534,7 @@ kvp_get_ip_address(int family, char *if_name, int op,
> > struct ifaddrs *ifap;
> > struct ifaddrs *curp;
> > int offset = 0;
> > + int sn_offset = 0;
> > const char *str;
> > int error = 0;
> > char *buffer;
> > @@ -594,12 +595,38 @@ kvp_get_ip_address(int family, char *if_name, int op,
> > * Gather info other than the IP address.
> > * IP address info will be gathered later.
> > */
> > - if (curp->ifa_addr->sa_family == AF_INET)
> > + if (curp->ifa_addr->sa_family == AF_INET) {
> > ip_buffer->addr_family |= ADDR_FAMILY_IPV4;
> > - else
> > + /*
> > + * Get subnet info.
> > + */
> > + error = kvp_process_ip_address(
> > + curp->ifa_netmask,
> > + AF_INET,
> > + (char *)
> > + ip_buffer->sub_net,
> > + length,
> > + &sn_offset);
> [...]
>
> This is barely readable; why don't you indent the arguments by just one
> extra tab?
Will do.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 1/1] Drivers: hv: Cleanup the guest ID computation
From: KY Srinivasan @ 2012-07-25 14:18 UTC (permalink / raw)
To: KY Srinivasan, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
virtualization@lists.osdl.org, olaf@aepfle.de, apw@canonical.com
In-Reply-To: <1343171518-15189-1-git-send-email-kys@microsoft.com>
This is the patch I sent out yesterday for Linux to clean up the guest ID mess. For FreeBSD, you can use the following constant:
HV_FREEBSD_VENDOR_ID 0x8200
and use the function to generate the ID appropriately. Larry, I will forward you the proposal for guest ID in a separate email.
Regards.
K. Y
> -----Original Message-----
> From: K. Y. Srinivasan [mailto:kys@microsoft.com]
> Sent: Tuesday, July 24, 2012 7:12 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com
> Cc: KY Srinivasan
> Subject: [PATCH 1/1] Drivers: hv: Cleanup the guest ID computation
>
> The current guest ID string in use in vmbus driver does not conform
> to the MSFT guidelines on guest ID. MSFT currently does not specify
> Linux specific guidelines. MSFT however has plans to publish Linux
> specific guidelines. This implementation conforms to the yet unpublished
> Linux specific guidelines for guest ID. This implementation also broadly
> conforms to the current guidelines as well.
>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/hv/hv.c | 9 +++++--
> drivers/hv/hyperv_vmbus.h | 48
> +++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 86f8885..771e24f 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -26,6 +26,7 @@
> #include <linux/slab.h>
> #include <linux/vmalloc.h>
> #include <linux/hyperv.h>
> +#include <linux/version.h>
> #include <asm/hyperv.h>
> #include "hyperv_vmbus.h"
>
> @@ -164,9 +165,11 @@ int hv_init(void)
>
> max_leaf = query_hypervisor_info();
>
> - /* Write our OS info */
> - wrmsrl(HV_X64_MSR_GUEST_OS_ID, HV_LINUX_GUEST_ID);
> - hv_context.guestid = HV_LINUX_GUEST_ID;
> + /*
> + * Write our OS ID.
> + */
> + hv_context.guestid = generate_guest_id(0, LINUX_VERSION_CODE, 0);
> + wrmsrl(HV_X64_MSR_GUEST_OS_ID, hv_context.guestid);
>
> /* See if the hypercall page is already set */
> rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 0614ff3..108a441 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -410,10 +410,50 @@ enum {
>
> #define HV_PRESENT_BIT 0x80000000
>
> -#define HV_LINUX_GUEST_ID_LO 0x00000000
> -#define HV_LINUX_GUEST_ID_HI 2976579765
> -#define HV_LINUX_GUEST_ID (((u64)HV_LINUX_GUEST_ID_HI << 32) |
> \
> - HV_LINUX_GUEST_ID_LO)
> +/*
> + * The guest OS needs to register the guest ID with the hypervisor.
> + * The guest ID is a 64 bit entity and the structure of this ID is
> + * specified in the Hyper-V specification:
> + *
> + * http://msdn.microsoft.com/en-us/library/windows/
> + * hardware/ff542653%28v=vs.85%29.aspx
> + *
> + * While the current guideline does not specify how Linux guest ID(s)
> + * need to be generated, our plan is to publish the guidelines for
> + * Linux and other guest operating systems that currently are hosted
> + * on Hyper-V. The implementation here conforms to this yet
> + * unpublished guidelines.
> + *
> + *
> + * Bit(s)
> + * 63 - Indicates if the OS is Open Source or not; 1 is Open Source
> + * 62:56 - Os Type; Linux is 0x100
> + * 55:48 - Distro specific identification
> + * 47:16 - Linux kernel version number
> + * 15:0 - Distro specific identification
> + *
> + *
> + */
> +
> +#define HV_LINUX_VENDOR_ID 0x8100
> +
> +/*
> + * Generate the guest ID based on the guideline described above.
> + */
> +
> +static inline __u64 generate_guest_id(__u8 d_info1, __u32 kernel_version,
> + __u16 d_info2)
> +{
> + __u64 guest_id = 0;
> +
> + guest_id = (((__u64)HV_LINUX_VENDOR_ID) << 48);
> + guest_id |= (((__u64)(d_info1)) << 48);
> + guest_id |= (((__u64)(kernel_version)) << 16);
> + guest_id |= ((__u64)(d_info2));
> +
> + return guest_id;
> +}
> +
>
> #define HV_CPU_POWER_MANAGEMENT (1 << 0)
> #define HV_RECOMMENDATIONS_MAX 4
> --
> 1.7.4.1
>
>
>
^ permalink raw reply
* Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
From: Ben Hutchings @ 2012-07-25 14:47 UTC (permalink / raw)
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, netdev@vger.kernel.org
In-Reply-To: <426367E2313C2449837CD2DE46E7EAF9236A8B61@SN2PRD0310MB382.namprd03.prod.outlook.com>
On Wed, Jul 25, 2012 at 02:10:05PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > Sent: Tuesday, July 24, 2012 9:11 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; netdev@vger.kernel.org
> > Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
> >
> > On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > > In preparation to implementing IP injection, cleanup the way we propagate
> > > and handle errors both in the driver as well as in the user level daemon.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---
> > > drivers/hv/hv_kvp.c | 112 +++++++++++++++++++++++++++++++++++++-
> > --------
> > > include/linux/hyperv.h | 17 +++++---
> > > tools/hv/hv_kvp_daemon.c | 70 +++++++++++++++-------------
> > > 3 files changed, 138 insertions(+), 61 deletions(-)
> > >
> > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> > > index 0012eed..9b7fc4a 100644
> > > --- a/drivers/hv/hv_kvp.c
> > > +++ b/drivers/hv/hv_kvp.c
> > [...]
> > > @@ -109,27 +154,52 @@ kvp_cn_callback(struct cn_msg *msg, struct
> > netlink_skb_parms *nsp)
> > > {
> > > struct hv_kvp_msg *message;
> > > struct hv_kvp_msg_enumerate *data;
> > > + int error = 0;
> > >
> > > message = (struct hv_kvp_msg *)msg->data;
> > > - switch (message->kvp_hdr.operation) {
> > > +
> > > + /*
> > > + * If we are negotiating the version information
> > > + * with the daemon; handle that first.
> > > + */
> > > +
> > > + if (in_hand_shake) {
> > > + if (kvp_handle_handshake(message))
> > > + in_hand_shake = false;
> > > + return;
> > > + }
> > > +
> > > + /*
> > > + * Based on the version of the daemon, we propagate errors from the
> > > + * daemon differently.
> > > + */
> > > +
> > > + data = &message->body.kvp_enum_data;
> > > +
> > > + switch (dm_reg_value) {
> > > case KVP_OP_REGISTER:
> > > - pr_info("KVP: user-mode registering done.\n");
> > > - kvp_register();
> > > - kvp_transaction.active = false;
> > > - hv_kvp_onchannelcallback(kvp_transaction.kvp_context);
> > > + /*
> > > + * Null string is used to pass back error condition.
> > > + */
> > > + if (!strlen(data->data.key))
> >
> > Do we know that the key is null-terminated here? Shouldn't we just
> > check whether data->data.key[0] == 0?
>
> Yes, currently we do return null string to indicate error.
[...]
So the kernel should assume userland input is always valid?
Ben.
--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus
^ permalink raw reply
* RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: KY Srinivasan @ 2012-07-25 14:48 UTC (permalink / raw)
To: Ben Hutchings
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
olaf@aepfle.de, apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <1343179491.5132.112.camel@deadeye.wl.decadent.org.uk>
Ben,
At the outset I want to thank you for taking the time to review this code. Given that Greg
has indicated that he will not be able to look at this patch set till 3.6 and the nature of review
comments I have gotten from you and others, I will re-spin this patch set to address all the
comments I have gotten to date. Specific responses to your comments are in-line.
Regards,
K. Y
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, July 24, 2012 9: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; netdev@vger.kernel.org
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
>
> On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > Implement the KVP verb - KVP_OP_SET_IP_INFO. This operation configures
> the
> > specified interface based on the given configuration. Since configuring
> > an interface is very distro specific, we invoke an external script to
> > configure the interface.
> [...]
> > +static int kvp_write_file(FILE *f, char *s1, char *s2, char *s3)
> > +{
> > + char str[256];
> > + int error;
> > +
> > + memset(str, 0, sizeof(str));
> > + strcat(str, s1);
> > + if (s2 != NULL)
> > + strcat(str, s2);
> > + strcat(str, "=");
> > + strcat(str, s3);
> > + strcat(str, "\n");
> > +
> > + error = fputs(str, f);
>
> This style of string pasting is crazy; have you never heard of
> fprintf()?
>
> [...]
> > + /*
> > + * Set the configuration for the specified interface with
> > + * the information provided. Since there is no standard
> > + * way to configure an interface, we will have an external
> > + * script that does the job of configuring the interface and
> > + * flushing the configuration.
> > + *
> > + * The parameters passed to this external script are:
> > + * 1. A configuration file that has the specified configuration.
> > + *
> > + * We will embed the name of the interface in the configuration
> > + * file: ifcfg-ethx (where ethx is the interface name).
> > + *
> > + * Here is the format of the ip configuration file:
> > + *
> > + * HWADDR=macaddr
>
> Is the interface supposed to be matched by name or by MAC address?
I do not dictate that. My plan was to package all the information I have about
the interface and the desired configuration in a file and invoke the external
distro specific script to do its magic. This external script is free to ignore
what it does not need.
>
> > + * BOOTPROTO=dhcp (dhcp enabled for the interface)
>
> The BOOTPROTO line may or may not appear.
>
> > + * NM_CONTROLLED=no (this interface will not be controlled by NM)
> > + * PEERDNS=yes
>
> I wonder what the point is of including constant lines in the file.
> What is the external script supposed to do if it these apparent
> constants change in future?
As you can see, I did my testing on a RHEL system and I was too lazy to create
a RHEL specific config file in the external script and so I ended up creating pretty much
the config file needed by RHEL in this daemon. All the external script had to do was to
simply copy this file into the right location and bring up the interface. So, if you prefer
that we not populate the config file with these constant lines, I can get rid of them.
As I noted earlier external scripts may not choose to use all the information in the file
that this daemon generates. What I have done here, simplifies the external script
at least for one distro.
>
> > + * IPADDR_x=ipaddr
> > + * NETMASK_x=netmask
> > + * GATEWAY_x=gateway
> > + * DNSx=dns
>
> A strangely familiar format...
>
> > + * IPV6 addresses will be tagged as IPV6ADDR, IPV6 gateway will be
> > + * tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as
> > + * IPV6NETMASK.
> > + */
> > +
> > + memset(if_file, 0, sizeof(if_file));
> > + strcat(if_file, "/var/opt/hyperv/ifcfg-");
>
> Like I said before about the key-value files, this should be under
> /var/lib if the daemon is included in a distribution. You should
> perhaps use a macro for the "/var/opt" part so it can be overridden
> depending on whether it's built as a distribution or add-on package.
I will make this a macro.
>
> > + strcat(if_file, if_name);
> > +
> > + file = fopen(if_file, "w");
> > +
> > + if (file == NULL) {
> > + syslog(LOG_ERR, "Failed to open config file");
> > + return HV_E_FAIL;
> > + }
> > +
> > + /*
> > + * First write out the MAC address.
> > + */
> > +
> > + mac_addr = kvp_if_name_to_mac(if_name);
> > + if (mac_addr == NULL) {
> > + error = HV_E_FAIL;
> > + goto setval_error;
> > + }
> > +
> > + error = kvp_write_file(file, "HWADDR", NULL, mac_addr);
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "ONBOOT", NULL, "yes");
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "IPV6INIT", NULL, "yes");
> > + if (error)
> > + goto setval_error;
> [...]
>
> This line isn't mentioned in the above comment.
>
> Ben.
>
> --
> Ben Hutchings
> If more than one person is responsible for a bug, no one is at fault.
^ permalink raw reply
* RE: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
From: KY Srinivasan @ 2012-07-25 14:51 UTC (permalink / raw)
To: Ben Hutchings
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
olaf@aepfle.de, apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <20120725144724.GD1894@decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Wednesday, July 25, 2012 10:47 AM
> 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; netdev@vger.kernel.org
> Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
>
> On Wed, Jul 25, 2012 at 02:10:05PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > > Sent: Tuesday, July 24, 2012 9:11 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; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
> > >
> > > On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > > > In preparation to implementing IP injection, cleanup the way we propagate
> > > > and handle errors both in the driver as well as in the user level daemon.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > ---
> > > > drivers/hv/hv_kvp.c | 112
> +++++++++++++++++++++++++++++++++++++-
> > > --------
> > > > include/linux/hyperv.h | 17 +++++---
> > > > tools/hv/hv_kvp_daemon.c | 70 +++++++++++++++-------------
> > > > 3 files changed, 138 insertions(+), 61 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> > > > index 0012eed..9b7fc4a 100644
> > > > --- a/drivers/hv/hv_kvp.c
> > > > +++ b/drivers/hv/hv_kvp.c
> > > [...]
> > > > @@ -109,27 +154,52 @@ kvp_cn_callback(struct cn_msg *msg, struct
> > > netlink_skb_parms *nsp)
> > > > {
> > > > struct hv_kvp_msg *message;
> > > > struct hv_kvp_msg_enumerate *data;
> > > > + int error = 0;
> > > >
> > > > message = (struct hv_kvp_msg *)msg->data;
> > > > - switch (message->kvp_hdr.operation) {
> > > > +
> > > > + /*
> > > > + * If we are negotiating the version information
> > > > + * with the daemon; handle that first.
> > > > + */
> > > > +
> > > > + if (in_hand_shake) {
> > > > + if (kvp_handle_handshake(message))
> > > > + in_hand_shake = false;
> > > > + return;
> > > > + }
> > > > +
> > > > + /*
> > > > + * Based on the version of the daemon, we propagate errors from the
> > > > + * daemon differently.
> > > > + */
> > > > +
> > > > + data = &message->body.kvp_enum_data;
> > > > +
> > > > + switch (dm_reg_value) {
> > > > case KVP_OP_REGISTER:
> > > > - pr_info("KVP: user-mode registering done.\n");
> > > > - kvp_register();
> > > > - kvp_transaction.active = false;
> > > > - hv_kvp_onchannelcallback(kvp_transaction.kvp_context);
> > > > + /*
> > > > + * Null string is used to pass back error condition.
> > > > + */
> > > > + if (!strlen(data->data.key))
> > >
> > > Do we know that the key is null-terminated here? Shouldn't we just
> > > check whether data->data.key[0] == 0?
> >
> > Yes, currently we do return null string to indicate error.
> [...]
>
> So the kernel should assume userland input is always valid?
Good point! This is the existing code and this patch-set cleans up all
the error handling and would not have this problem.
K. Y
^ permalink raw reply
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Nicholas A. Bellinger @ 2012-07-25 21:10 UTC (permalink / raw)
To: Avi Kivity
Cc: Jens Axboe, linux-scsi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, Paolo Bonzini, Zhi Yong Wu, Christoph Hellwig,
Stefan Hajnoczi
In-Reply-To: <500FF05B.8060602@redhat.com>
On Wed, 2012-07-25 at 16:10 +0300, Avi Kivity wrote:
> On 07/24/2012 11:45 PM, Nicholas A. Bellinger wrote:
>
> >> > diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
> >> > index e942df9..3d5378f 100644
> >> > --- a/drivers/vhost/tcm_vhost.h
> >> > +++ b/drivers/vhost/tcm_vhost.h
> >> > @@ -80,7 +80,17 @@ struct tcm_vhost_tport {
> >> >
> >> > #include <linux/vhost.h>
> >> >
> >> > +/*
> >> > + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> >> > + *
> >> > + * ABI Rev 0: All pre 2012 revisions used by prototype out-of-tree code
> >> > + * ABI Rev 1: 2012 version for v3.6 kernel merge candiate
> >> > + */
>
>
> If it's out of tree, why consider it at all? Put a stable ABI in tree
> and extend it in compatible ways.
>
>
This comment was supposed to convey that ABI=0 vhost-scsi userspace code
is not supported with tcm_vhost mainline code.
But obviously that was not clear enough here. Updating the comment to
reflect to make this clear.
So the main question here was if it's fine to start with ABI=1, and
require >= ABI=1 for all vhost-scsi userspace code to function with
tcm_vhost.
The idea was to avoid confusion for the ABI=0 vhost-scsi code that's
been floating around for the last 2 years.
--nab
^ permalink raw reply
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Nicholas A. Bellinger @ 2012-07-25 21:14 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Jens Axboe, linux-scsi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, Paolo Bonzini, Zhi Yong Wu, Christoph Hellwig,
Avi Kivity
In-Reply-To: <20120725115552.GB11098@stefanha-thinkpad.localdomain>
On Wed, 2012-07-25 at 12:55 +0100, Stefan Hajnoczi wrote:
> On Tue, Jul 24, 2012 at 01:45:24PM -0700, Nicholas A. Bellinger wrote:
> > On Mon, 2012-07-23 at 18:56 -0700, Greg Kroah-Hartman wrote:
> > > On Tue, Jul 24, 2012 at 01:26:20AM +0000, Nicholas A. Bellinger wrote:
> > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
<SNIP>
> >
> > > > diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
> > > > index e942df9..3d5378f 100644
> > > > --- a/drivers/vhost/tcm_vhost.h
> > > > +++ b/drivers/vhost/tcm_vhost.h
> > > > @@ -80,7 +80,17 @@ struct tcm_vhost_tport {
> > > >
> > > > #include <linux/vhost.h>
> > > >
> > > > +/*
> > > > + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> > > > + *
> > > > + * ABI Rev 0: All pre 2012 revisions used by prototype out-of-tree code
> > > > + * ABI Rev 1: 2012 version for v3.6 kernel merge candiate
> > > > + */
> > > > +
> > > > +#define VHOST_SCSI_ABI_VERSION 1
> > > > +
> > > > struct vhost_scsi_target {
> > > > + int abi_version;
> > > > unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
> > > > unsigned short vhost_tpgt;
> > > > };
> > > > @@ -88,3 +98,4 @@ struct vhost_scsi_target {
> > > > /* VHOST_SCSI specific defines */
> > > > #define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> > > > #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> > > > +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> > >
> > > No, you just broke the ABI for version "0" here, that's not how you do
> > > this at all.
> > >
> >
> > The intention of this patch is use ABI=1 as a starting point for
> > tcm_vhost moving forward, with no back-wards compat for the ABI=0
> > prototype userspace code because:
> >
> > - It's based on a slightly older version of QEMU (updating the QEMU series now)
> > - It does not have an GET_ABI_VERSION ioctl cmd (that starts with ABI=1)
> > - It has a small user-base of target + virtio-scsi developers
> >
> > So I did consider just starting from ABI=0, but figured this would help
> > reduce the confusion for QEMU userspace wrt to the vhost-scsi code
> > that's been floating around out-of-tree for the last 2 years.
>
> There is no real user base beyond the handful of people who have hacked
> on this. Adding the GET_ABI_VERSION ioctl() at this stage is fine,
> especially considering that the userspace code that talks to tcm_vhost
> isn't in mainline in userspace yet either.
Do you have a preference for a VHOST_SCSI_ABI_VERSION starting point
here..?
I thought that v1 would be helpful to avoid confusion with the older
userspace code, but don't really have a strong opinion either way..
Let me know what you'd prefer here, and I'll make the changes to
tcm_vhost + vhost-scsi patch series accordingly.
Thanks!
--nab
^ permalink raw reply
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Stefan Hajnoczi @ 2012-07-25 22:35 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, linux-scsi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, Paolo Bonzini, Zhi Yong Wu, Christoph Hellwig,
Avi Kivity
In-Reply-To: <1343250890.1813.173.camel@haakon2.linux-iscsi.org>
On Wed, Jul 25, 2012 at 02:14:50PM -0700, Nicholas A. Bellinger wrote:
> On Wed, 2012-07-25 at 12:55 +0100, Stefan Hajnoczi wrote:
> > On Tue, Jul 24, 2012 at 01:45:24PM -0700, Nicholas A. Bellinger wrote:
> > > On Mon, 2012-07-23 at 18:56 -0700, Greg Kroah-Hartman wrote:
> > > > On Tue, Jul 24, 2012 at 01:26:20AM +0000, Nicholas A. Bellinger wrote:
> > > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> <SNIP>
>
> > >
> > > > > diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
> > > > > index e942df9..3d5378f 100644
> > > > > --- a/drivers/vhost/tcm_vhost.h
> > > > > +++ b/drivers/vhost/tcm_vhost.h
> > > > > @@ -80,7 +80,17 @@ struct tcm_vhost_tport {
> > > > >
> > > > > #include <linux/vhost.h>
> > > > >
> > > > > +/*
> > > > > + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> > > > > + *
> > > > > + * ABI Rev 0: All pre 2012 revisions used by prototype out-of-tree code
> > > > > + * ABI Rev 1: 2012 version for v3.6 kernel merge candiate
> > > > > + */
> > > > > +
> > > > > +#define VHOST_SCSI_ABI_VERSION 1
> > > > > +
> > > > > struct vhost_scsi_target {
> > > > > + int abi_version;
> > > > > unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
> > > > > unsigned short vhost_tpgt;
> > > > > };
> > > > > @@ -88,3 +98,4 @@ struct vhost_scsi_target {
> > > > > /* VHOST_SCSI specific defines */
> > > > > #define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> > > > > #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> > > > > +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> > > >
> > > > No, you just broke the ABI for version "0" here, that's not how you do
> > > > this at all.
> > > >
> > >
> > > The intention of this patch is use ABI=1 as a starting point for
> > > tcm_vhost moving forward, with no back-wards compat for the ABI=0
> > > prototype userspace code because:
> > >
> > > - It's based on a slightly older version of QEMU (updating the QEMU series now)
> > > - It does not have an GET_ABI_VERSION ioctl cmd (that starts with ABI=1)
> > > - It has a small user-base of target + virtio-scsi developers
> > >
> > > So I did consider just starting from ABI=0, but figured this would help
> > > reduce the confusion for QEMU userspace wrt to the vhost-scsi code
> > > that's been floating around out-of-tree for the last 2 years.
> >
> > There is no real user base beyond the handful of people who have hacked
> > on this. Adding the GET_ABI_VERSION ioctl() at this stage is fine,
> > especially considering that the userspace code that talks to tcm_vhost
> > isn't in mainline in userspace yet either.
>
>
> Do you have a preference for a VHOST_SCSI_ABI_VERSION starting point
> here..?
>
> I thought that v1 would be helpful to avoid confusion with the older
> userspace code, but don't really have a strong opinion either way..
>
> Let me know what you'd prefer here, and I'll make the changes to
> tcm_vhost + vhost-scsi patch series accordingly.
I don't think 0 for out-of-tree is needed. I'd start at 0 but either
way is okay.
The main thing I would like to confirm is that this only versions the
tcm_vhost ioctls? In that case a single version number works.
Stefan
^ permalink raw reply
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Nicholas A. Bellinger @ 2012-07-26 2:34 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Jens Axboe, linux-scsi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, Paolo Bonzini, Zhi Yong Wu, Christoph Hellwig,
Avi Kivity
In-Reply-To: <20120725223559.GA4786@stefanha-thinkpad.localdomain>
On Wed, 2012-07-25 at 23:35 +0100, Stefan Hajnoczi wrote:
> On Wed, Jul 25, 2012 at 02:14:50PM -0700, Nicholas A. Bellinger wrote:
> > On Wed, 2012-07-25 at 12:55 +0100, Stefan Hajnoczi wrote:
> > > On Tue, Jul 24, 2012 at 01:45:24PM -0700, Nicholas A. Bellinger wrote:
> > > > On Mon, 2012-07-23 at 18:56 -0700, Greg Kroah-Hartman wrote:
> > > > > On Tue, Jul 24, 2012 at 01:26:20AM +0000, Nicholas A. Bellinger wrote:
> > > > > > From: Nicholas Bellinger <nab@linux-iscsi.org>
<SNIP>
> > > > The intention of this patch is use ABI=1 as a starting point for
> > > > tcm_vhost moving forward, with no back-wards compat for the ABI=0
> > > > prototype userspace code because:
> > > >
> > > > - It's based on a slightly older version of QEMU (updating the QEMU series now)
> > > > - It does not have an GET_ABI_VERSION ioctl cmd (that starts with ABI=1)
> > > > - It has a small user-base of target + virtio-scsi developers
> > > >
> > > > So I did consider just starting from ABI=0, but figured this would help
> > > > reduce the confusion for QEMU userspace wrt to the vhost-scsi code
> > > > that's been floating around out-of-tree for the last 2 years.
> > >
> > > There is no real user base beyond the handful of people who have hacked
> > > on this. Adding the GET_ABI_VERSION ioctl() at this stage is fine,
> > > especially considering that the userspace code that talks to tcm_vhost
> > > isn't in mainline in userspace yet either.
> >
> >
> > Do you have a preference for a VHOST_SCSI_ABI_VERSION starting point
> > here..?
> >
> > I thought that v1 would be helpful to avoid confusion with the older
> > userspace code, but don't really have a strong opinion either way..
> >
> > Let me know what you'd prefer here, and I'll make the changes to
> > tcm_vhost + vhost-scsi patch series accordingly.
>
> I don't think 0 for out-of-tree is needed. I'd start at 0 but either
> way is okay.
>
<nod>
In that case, respinning a -v5 for tcm_vhost to start from ABI=0 and
will post an updated patch shortly.
> The main thing I would like to confirm is that this only versions the
> tcm_vhost ioctls? In that case a single version number works.
>
Correct, the GET_ABI_VERSION call is only intended to identify the
changing of tcm_vhost ioctls.
^ permalink raw reply
* Re: [net-next RFC V5 2/5] virtio_ring: move queue_index to vring_virtqueue
From: Paolo Bonzini @ 2012-07-26 8:20 UTC (permalink / raw)
To: Sasha Levin
Cc: krkumar2, habanero, kvm, mst, netdev, mashirle, linux-kernel,
virtualization, edumazet, tahm, jwhan, davem, sri
In-Reply-To: <1341488454.18786.15.camel@lappy>
Il 05/07/2012 13:40, Sasha Levin ha scritto:
> @@ -275,7 +274,7 @@ static void vm_del_vq(struct virtqueue *vq)
> vring_del_virtqueue(vq);
>
> /* Select and deactivate the queue */
> - writel(info->queue_index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
> + writel(virtqueue_get_queue_index(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
> writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
>
This accesses vq after vring_del_virtqueue has freed it.
Paolo
^ permalink raw reply
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Avi Kivity @ 2012-07-26 8:58 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, linux-scsi, Paolo Bonzini, Zhi Yong Wu,
Christoph Hellwig
In-Reply-To: <1343270061.1813.181.camel@haakon2.linux-iscsi.org>
On 07/26/2012 05:34 AM, Nicholas A. Bellinger wrote:
>
> In that case, respinning a -v5 for tcm_vhost to start from ABI=0 and
> will post an updated patch shortly.
>
>> The main thing I would like to confirm is that this only versions the
>> tcm_vhost ioctls? In that case a single version number works.
>>
>
> Correct, the GET_ABI_VERSION call is only intended to identify the
> changing of tcm_vhost ioctls.
Why use version numbers at all?
Feature maps are better, because you can get very fine granularity.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: Re: Re: [RFC PATCH 0/6] virtio-trace: Support virtio-trace
From: Stefan Hajnoczi @ 2012-07-26 10:52 UTC (permalink / raw)
To: Yoshihiro YUNOMAE
Cc: Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman,
Frederic Weisbecker, Dhaval Giani, linux-kernel, Borislav Petkov,
qemu-devel, Amit Shah, Franch Ch. Eigler, Ingo Molnar,
Mathieu Desnoyers, Steven Rostedt, Anthony Liguori,
yrl.pp-manager.tt, Masami Hiramatsu, virtualization,
Srikar Dronamraju
In-Reply-To: <500FB8A4.6060503@hitachi.com>
On Wed, Jul 25, 2012 at 10:13 AM, Yoshihiro YUNOMAE
<yoshihiro.yunomae.ez@hitachi.com> wrote:
> Hi Stefan,
>
>
> (2012/07/24 22:41), Stefan Hajnoczi wrote:
>>
>> On Tue, Jul 24, 2012 at 12:19 PM, Yoshihiro YUNOMAE
>> <yoshihiro.yunomae.ez@hitachi.com> wrote:
>>>>>
>>>>> Are you using text formatted ftrace?
>>>
>>> No, currently using raw format, but we'd like to reformat it in text.
>>
>>
>> Capturing the info necessary to translate numbers into symbols is one
>> of the problems of host<->guest tracing so I'm curious how you handle
>> this :).
>
>
> Right, your consideration is true.
>
>
>> Apologies for my lack of ftrace knowledge but how useful is the raw
>> tracing data on the host? How do you pretty-print it in
>> human-readable form?
>
>
> perf and trace-cmd can actually translate raw-formatted trace data to
> text-formatted trace data by using information of kernel or trace
> format under tracing/events directory in debugfs. In the same way, if
> the information of a guest is exported to a host, we can translate
> raw trace data of a guest to text trace data on a host. We will use
> 9pfs to export that.
Thanks, it's clear now :).
Stefan
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC PATCH 0/6] virtio-trace: Support virtio-trace
From: Amit Shah @ 2012-07-26 11:35 UTC (permalink / raw)
To: Yoshihiro YUNOMAE
Cc: Arnd Bergmann, Greg Kroah-Hartman, Frederic Weisbecker,
qemu-devel, Borislav Petkov, linux-kernel, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, yrl.pp-manager.tt, virtualization
In-Reply-To: <20120724023657.6600.52706.stgit@ltc189.sdl.hitachi.co.jp>
On (Tue) 24 Jul 2012 [11:36:57], Yoshihiro YUNOMAE wrote:
> Hi All,
>
> The following patch set provides a low-overhead system for collecting kernel
> tracing data of guests by a host in a virtualization environment.
>
> A guest OS generally shares some devices with other guests or a host, so
> reasons of any problems occurring in a guest may be from other guests or a host.
> Then, to collect some tracing data of a number of guests and a host is needed
> when some problems occur in a virtualization environment. One of methods to
> realize that is to collect tracing data of guests in a host. To do this, network
> is generally used. However, high load will be taken to applications on guests
> using network I/O because there are many network stack layers. Therefore,
> a communication method for collecting the data without using network is needed.
>
> We submitted a patch set of "IVRing", a ring-buffer driver constructed on
> Inter-VM shared memory (IVShmem), to LKML http://lwn.net/Articles/500304/ in
> this June. IVRing and the IVRing reader use POSIX shared memory each other
> without using network, so a low-overhead system for collecting guest tracing
> data is realized. However, this patch set has some problems as follows:
> - use IVShmem instead of virtio
> - create a new ring-buffer without using existing ring-buffer in kernel
> - scalability
> -- not support SMP environment
> -- buffer size limitation
> -- not support live migration (maybe difficult for realize this)
>
> Therefore, we propose a new system "virtio-trace", which uses enhanced
> virtio-serial and existing ring-buffer of ftrace, for collecting guest kernel
> tracing data. In this system, there are 5 main components:
> (1) Ring-buffer of ftrace in a guest
> - When trace agent reads ring-buffer, a page is removed from ring-buffer.
> (2) Trace agent in the guest
> - Splice the page of ring-buffer to read_pipe using splice() without
> memory copying. Then, the page is spliced from write_pipe to virtio
> without memory copying.
I really like the splicing idea.
> (3) Virtio-console driver in the guest
> - Pass the page to virtio-ring
> (4) Virtio-serial bus in QEMU
> - Copy the page to kernel pipe
> (5) Reader in the host
> - Read guest tracing data via FIFO(named pipe)
So will this be useful only if guest and host run the same kernel?
I'd like to see the host kernel not being used at all -- collect all
relevant info from the guest and send it out to qemu, where it can be
consumed directly by apps driving the tracing.
> ***Evaluation***
> When a host collects tracing data of a guest, the performance of using
> virtio-trace is compared with that of using native(just running ftrace),
> IVRing, and virtio-serial(normal method of read/write).
Why is tracing performance-sensitive? i.e. why try to optimise this
at all?
> <environment>
> The overview of this evaluation is as follows:
> (a) A guest on a KVM is prepared.
> - The guest is dedicated one physical CPU as a virtual CPU(VCPU).
>
> (b) The guest starts to write tracing data to ring-buffer of ftrace.
> - The probe points are all trace points of sched, timer, and kmem.
>
> (c) Writing trace data, dhrystone 2 in UNIX bench is executed as a benchmark
> tool in the guest.
> - Dhrystone 2 intends system performance by repeating integer arithmetic
> as a score.
> - Since higher score equals to better system performance, if the score
> decrease based on bare environment, it indicates that any operation
> disturbs the integer arithmetic. Then, we define the overhead of
> transporting trace data is calculated as follows:
> OVERHEAD = (1 - SCORE_OF_A_METHOD/NATIVE_SCORE) * 100.
>
> The performance of each method is compared as follows:
> [1] Native
> - only recording trace data to ring-buffer on a guest
> [2] Virtio-trace
> - running a trace agent on a guest
> - a reader on a host opens FIFO using cat command
> [3] IVRing
> - A SystemTap script in a guest records trace data to IVRing.
> -- probe points are same as ftrace.
> [4] Virtio-serial(normal)
> - A reader(using cat) on a guest output trace data to a host using
> standard output via virtio-serial.
>
> Other information is as follows:
> - host
> kernel: 3.3.7-1 (Fedora16)
> CPU: Intel Xeon x5660@2.80GHz(12core)
> Memory: 48GB
>
> - guest(only booting one guest)
> kernel: 3.5.0-rc4+ (Fedora16)
> CPU: 1VCPU(dedicated)
> Memory: 1GB
>
> <result>
> 3 patterns based on the bare environment were indicated as follows:
> Scores overhead against [0] Native
> [0] Native: 28807569.5 -
> [1] Virtio-trace: 28685049.5 0.43%
> [2] IVRing: 28418595.5 1.35%
> [3] Virtio-serial: 13262258.7 53.96%
>
>
> ***Just enhancement ideas***
> - Support for trace-cmd
> - Support for 9pfs protocol
> - Support for non-blocking mode in QEMU
There were patches long back (by me) to make chardevs non-blocking but
they didn't make it upstream. Fedora carries them, if you want to try
out. Though we want to converge on a reasonable solution that's
acceptable upstream as well. Just that no one's working on it
currently. Any help here will be appreciated.
> - Make "vhost-serial"
I need to understand a) why it's perf-critical, and b) why should the
host be involved at all, to comment on these.
Thanks,
Amit
^ permalink raw reply
* [PATCH 0/2] virtio-scsi fixes for 3.6
From: Paolo Bonzini @ 2012-07-26 13:28 UTC (permalink / raw)
To: linux-kernel; +Cc: JBottomley, kvm, linux-scsi, virtualization
James,
patch 1 fixes scanning of LUNs whose number is greater than 255. QEMU
passes a max_lun of 16383 (because it uses SAM numbering) but in Linux
it must become 32768 (because LUNs above 255 are "relocated" to 16640).
Patch 2 is a resubmission of the patch for online resizing of virtio-scsi
LUNs, which needs to be rebased.
LUNs above 255 now work for all of scanning, hotplug, hotunplug and
resize.
Thanks,
Paolo
Paolo Bonzini (2):
virtio-scsi: fix LUNs greater than 255
virtio-scsi: support online resizing of disks
drivers/scsi/virtio_scsi.c | 37 +++++++++++++++++++++++++++++++++++--
include/linux/virtio_scsi.h | 2 ++
2 files changed, 37 insertions(+), 2 deletions(-)
^ permalink raw reply
* [PATCH 1/2] virtio-scsi: fix LUNs greater than 255
From: Paolo Bonzini @ 2012-07-26 13:28 UTC (permalink / raw)
To: linux-kernel; +Cc: stable, JBottomley, kvm, linux-scsi, virtualization
In-Reply-To: <1343309288-32615-1-git-send-email-pbonzini@redhat.com>
virtio-scsi needs to report LUNs greater than 256 using the "flat"
format. Because the Linux SCSI layer just maps the SCSI LUN to
an u32, without any parsing, these end up in the range from 16640
to 32767. Fix max_lun to account for the possibility that logical
unit numbers are encoded with the "flat" format.
Cc: <stable@vger.kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
drivers/scsi/virtio_scsi.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index c7030fb..8b6b927 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -677,7 +677,11 @@ static int __devinit virtscsi_probe(struct virtio_device *vdev)
cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1;
shost->cmd_per_lun = min_t(u32, cmd_per_lun, shost->can_queue);
shost->max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0xFFFF;
- shost->max_lun = virtscsi_config_get(vdev, max_lun) + 1;
+
+ /* LUNs > 256 are reported with format 1, so they go in the range
+ * 16640-32767.
+ */
+ shost->max_lun = virtscsi_config_get(vdev, max_lun) + 1 + 0x4000;
shost->max_id = num_targets;
shost->max_channel = 0;
shost->max_cmd_len = VIRTIO_SCSI_CDB_SIZE;
--
1.7.1
^ permalink raw reply related
* [PATCH 2/2] virtio-scsi: support online resizing of disks
From: Paolo Bonzini @ 2012-07-26 13:28 UTC (permalink / raw)
To: linux-kernel; +Cc: JBottomley, kvm, linux-scsi, virtualization
In-Reply-To: <1343309288-32615-1-git-send-email-pbonzini@redhat.com>
Support the LUN parameter change event. Currently, the host fires this event
when the capacity of a disk is changed from the virtual machine monitor.
The resize then appears in the kernel log like this:
sd 0:0:0:0: [sda] 46137344 512-byte logical blocks: (23.6 GB/22.0 GIb)
sda: detected capacity change from 22548578304 to 23622320128
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
drivers/scsi/virtio_scsi.c | 31 ++++++++++++++++++++++++++++++-
include/linux/virtio_scsi.h | 2 ++
2 files changed, 32 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 8b6b927..c1a5e60 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -279,6 +279,31 @@ static void virtscsi_handle_transport_reset(struct virtio_scsi *vscsi,
}
}
+static void virtscsi_handle_param_change(struct virtio_scsi *vscsi,
+ struct virtio_scsi_event *event)
+{
+ struct scsi_device *sdev;
+ struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev);
+ unsigned int target = event->lun[1];
+ unsigned int lun = (event->lun[2] << 8) | event->lun[3];
+ u8 asc = event->reason & 255;
+ u8 ascq = event->reason >> 8;
+
+ sdev = scsi_device_lookup(shost, 0, target, lun);
+ if (!sdev) {
+ pr_err("SCSI device %d 0 %d %d not found\n",
+ shost->host_no, target, lun);
+ return;
+ }
+
+ /* Handle "Parameters changed", "Mode parameters changed", and
+ "Capacity data has changed". */
+ if (asc == 0x2a && (ascq == 0x00 || ascq == 0x01 || ascq == 0x09))
+ scsi_rescan_device(&sdev->sdev_gendev);
+
+ scsi_device_put(sdev);
+}
+
static void virtscsi_handle_event(struct work_struct *work)
{
struct virtio_scsi_event_node *event_node =
@@ -297,6 +322,9 @@ static void virtscsi_handle_event(struct work_struct *work)
case VIRTIO_SCSI_T_TRANSPORT_RESET:
virtscsi_handle_transport_reset(vscsi, event);
break;
+ case VIRTIO_SCSI_T_PARAM_CHANGE:
+ virtscsi_handle_param_change(vscsi, event);
+ break;
default:
pr_err("Unsupport virtio scsi event %x\n", event->event);
}
@@ -737,7 +765,8 @@ static struct virtio_device_id id_table[] = {
};
static unsigned int features[] = {
- VIRTIO_SCSI_F_HOTPLUG
+ VIRTIO_SCSI_F_HOTPLUG,
+ VIRTIO_SCSI_F_CHANGE,
};
static struct virtio_driver virtio_scsi_driver = {
diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h
index dc8d305..d6b4440 100644
--- a/include/linux/virtio_scsi.h
+++ b/include/linux/virtio_scsi.h
@@ -72,6 +72,7 @@ struct virtio_scsi_config {
/* Feature Bits */
#define VIRTIO_SCSI_F_INOUT 0
#define VIRTIO_SCSI_F_HOTPLUG 1
+#define VIRTIO_SCSI_F_CHANGE 2
/* Response codes */
#define VIRTIO_SCSI_S_OK 0
@@ -108,6 +109,7 @@ struct virtio_scsi_config {
#define VIRTIO_SCSI_T_NO_EVENT 0
#define VIRTIO_SCSI_T_TRANSPORT_RESET 1
#define VIRTIO_SCSI_T_ASYNC_NOTIFY 2
+#define VIRTIO_SCSI_T_PARAM_CHANGE 3
/* Reasons of transport reset event */
#define VIRTIO_SCSI_EVT_RESET_HARD 0
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION
From: Nicholas A. Bellinger @ 2012-07-26 20:15 UTC (permalink / raw)
To: Avi Kivity
Cc: Jens Axboe, Stefan Hajnoczi, kvm-devel, Michael S. Tsirkin,
Greg Kroah-Hartman, qemu-devel, lf-virt, Anthony Liguori,
target-devel, linux-scsi, Paolo Bonzini, Zhi Yong Wu,
Christoph Hellwig
In-Reply-To: <5011069A.2080203@redhat.com>
On Thu, 2012-07-26 at 11:58 +0300, Avi Kivity wrote:
> On 07/26/2012 05:34 AM, Nicholas A. Bellinger wrote:
> >
> > In that case, respinning a -v5 for tcm_vhost to start from ABI=0 and
> > will post an updated patch shortly.
> >
> >> The main thing I would like to confirm is that this only versions the
> >> tcm_vhost ioctls? In that case a single version number works.
> >>
> >
> > Correct, the GET_ABI_VERSION call is only intended to identify the
> > changing of tcm_vhost ioctls.
>
> Why use version numbers at all?
>
This was a request by QEMU folks..
> Feature maps are better, because you can get very fine granularity.
>
>
Example..? If there is a better way to handle ioctl compat I'd
certainly like to hear about it.
^ permalink raw reply
* [vmw_vmci 00/11] VMCI for Linux
From: Andrew Stiegmann (stieg) @ 2012-07-26 23:39 UTC (permalink / raw)
To: linux-kernel, virtualization
Cc: pv-drivers, vm-crosstalk, Andrew Stiegmann (stieg), cschamp,
gregkh
In an effort to improve the out-of-the-box experience with Linux
kernels for VMware users, VMware is working on readying the Virtual
Machine Communication Interface (vmw_vmci) and VMCI Sockets
(vmw_vsock) kernel modules for inclusion in the Linux kernel. The
purpose of this post is to acquire feedback on the vmw_vmci kernel
module. The vmw_vsock kernel module will be presented in a later post.
VMCI allows virtual machines to communicate with host kernel modules
and the VMware hypervisors. User level applications both in a virtual
machine and on the host can use vmw_vmci through VMCI Sockets, a socket
address family designed to be compatible with UDP and TCP at the
interface level. Today, VMCI and VMCI Sockets are used by the VMware
shared folders (HGFS) and various VMware Tools components inside the
guest for zero-config, network-less access to VMware host services. In
addition to this, VMware's users are using VMCI Sockets for various
applications, where network access of the virtual machine is
restricted or non-existent. Examples of this are VMs communicating
with device proxies for proprietary hardware running as host
applications and automated testing of applications running within
virtual machines.
In a virtual machine, VMCI is exposed as a regular PCI device. The
primary communication mechanisms supported are a point-to-point
bidirectional transport based on a pair of memory-mapped queues, and
asynchronous notifications in the form of datagrams and
doorbells. These features are available to kernel level components
such as HGFS and VMCI Sockets through the VMCI kernel API. In addition
to this, the VMCI kernel API provides support for receiving events
related to the state of the VMCI communication channels, and the
virtual machine itself.
Outside the virtual machine, the host side support of the VMCI kernel
module makes the same VMCI kernel API available to VMCI endpoints on
the host. In addition to this, the host side manages each VMCI device
in a virtual machine through a context object. This context object
serves to identify the virtual machine for communication, and to track
the resource consumption of the given VMCI device. Both operations
related to communication between the virtual machine and the host
kernel, and those related to the management of the VMCI device state
in the host kernel, are invoked by the user level component of the
hypervisor through a set of ioctls on the VMCI device node. To
provide seamless support for nested virtualization, where a virtual
machine may use both a VMCI PCI device to talk to its hypervisor, and
the VMCI host side support to run nested virtual machines, the VMCI
host and virtual machine support are combined in a single kernel
module.
For additional information about the use of VMCI and in particular
VMCI Sockets, please refer to the VMCI Socket Programming Guide
available at https://www.vmware.com/support/developer/vmci-sdk/.
Andrew Stiegmann (stieg) (11):
Apply VMCI context code
Apply VMCI datagram code
Apply VMCI doorbell code
Apply VMCI driver code
Apply VMCI event code
Apply dynamic array code
Apply VMCI hash table
Apply VMCI queue pairs
Apply VMCI resource code
Apply vmci routing code
Apply the header code to make VMCI build
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/vmw_vmci/Kconfig | 16 +
drivers/misc/vmw_vmci/Makefile | 43 +
drivers/misc/vmw_vmci/vmci_common_int.h | 58 +
drivers/misc/vmw_vmci/vmci_context.c | 1269 +++++++++++
drivers/misc/vmw_vmci/vmci_context.h | 161 ++
drivers/misc/vmw_vmci/vmci_datagram.c | 586 +++++
drivers/misc/vmw_vmci/vmci_datagram.h | 56 +
drivers/misc/vmw_vmci/vmci_doorbell.c | 751 ++++++
drivers/misc/vmw_vmci/vmci_doorbell.h | 57 +
drivers/misc/vmw_vmci/vmci_driver.c | 2298 +++++++++++++++++++
drivers/misc/vmw_vmci/vmci_driver.h | 52 +
drivers/misc/vmw_vmci/vmci_event.c | 451 ++++
drivers/misc/vmw_vmci/vmci_event.h | 29 +
drivers/misc/vmw_vmci/vmci_handle_array.c | 174 ++
drivers/misc/vmw_vmci/vmci_handle_array.h | 50 +
drivers/misc/vmw_vmci/vmci_hash_table.c | 332 +++
drivers/misc/vmw_vmci/vmci_hash_table.h | 56 +
drivers/misc/vmw_vmci/vmci_queue_pair.c | 3548 +++++++++++++++++++++++++++++
drivers/misc/vmw_vmci/vmci_queue_pair.h | 182 ++
drivers/misc/vmw_vmci/vmci_resource.c | 194 ++
drivers/misc/vmw_vmci/vmci_resource.h | 62 +
drivers/misc/vmw_vmci/vmci_route.c | 241 ++
drivers/misc/vmw_vmci/vmci_route.h | 34 +
include/linux/vmw_vmci_api.h | 89 +
include/linux/vmw_vmci_defs.h | 921 ++++++++
27 files changed, 11712 insertions(+), 0 deletions(-)
create mode 100644 drivers/misc/vmw_vmci/Kconfig
create mode 100644 drivers/misc/vmw_vmci/Makefile
create mode 100644 drivers/misc/vmw_vmci/vmci_common_int.h
create mode 100644 drivers/misc/vmw_vmci/vmci_context.c
create mode 100644 drivers/misc/vmw_vmci/vmci_context.h
create mode 100644 drivers/misc/vmw_vmci/vmci_datagram.c
create mode 100644 drivers/misc/vmw_vmci/vmci_datagram.h
create mode 100644 drivers/misc/vmw_vmci/vmci_doorbell.c
create mode 100644 drivers/misc/vmw_vmci/vmci_doorbell.h
create mode 100644 drivers/misc/vmw_vmci/vmci_driver.c
create mode 100644 drivers/misc/vmw_vmci/vmci_driver.h
create mode 100644 drivers/misc/vmw_vmci/vmci_event.c
create mode 100644 drivers/misc/vmw_vmci/vmci_event.h
create mode 100644 drivers/misc/vmw_vmci/vmci_handle_array.c
create mode 100644 drivers/misc/vmw_vmci/vmci_handle_array.h
create mode 100644 drivers/misc/vmw_vmci/vmci_hash_table.c
create mode 100644 drivers/misc/vmw_vmci/vmci_hash_table.h
create mode 100644 drivers/misc/vmw_vmci/vmci_queue_pair.c
create mode 100644 drivers/misc/vmw_vmci/vmci_queue_pair.h
create mode 100644 drivers/misc/vmw_vmci/vmci_resource.c
create mode 100644 drivers/misc/vmw_vmci/vmci_resource.h
create mode 100644 drivers/misc/vmw_vmci/vmci_route.c
create mode 100644 drivers/misc/vmw_vmci/vmci_route.h
create mode 100644 include/linux/vmw_vmci_api.h
create mode 100644 include/linux/vmw_vmci_defs.h
^ 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