Re: [PATCH 2/3] S390: Add virtio hotplug add support
On Mon, 13 Sep 2010 13:05:57 +0930 Rusty Russell ru...@rustcorp.com.au wrote: On Sun, 12 Sep 2010 06:30:43 pm Avi Kivity wrote: On 09/12/2010 02:42 AM, Alexander Graf wrote: On 24.08.2010, at 15:48, Alexander Graf wrote: The one big missing feature in s390-virtio was hotplugging. This is no more. This patch implements hotplug add support, so you can on the fly add new devices in the guest. Keep in mind that this needs a patch for qemu to actually leverage the functionality. Signed-off-by: Alexander Grafag...@suse.de ping (on the patch set)? Actually Marcelo applied it. But the natural place for it is Rusty's virtio tree. Rusty, if you want to take it, let me know and I'll drop it from kvm.git. I thought it would be in the s390 tree, which is why I didn't take it... But I'm *always* happy to let do the work! I didn't pick them up after I saw that Marcelo took them. If others want to do the work, be my guest.. -- blue skies, Martin. Reality continues to ruin my life. - Calvin. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On 09/13/2010 09:41 AM, Martin Schwidefsky wrote: Actually Marcelo applied it. But the natural place for it is Rusty's virtio tree. Rusty, if you want to take it, let me know and I'll drop it from kvm.git. I thought it would be in the s390 tree, which is why I didn't take it... But I'm *always* happy to let do the work! I didn't pick them up after I saw that Marcelo took them. If others want to do the work, be my guest.. I just hope that all this generosity doesn't lead to merge conflicts later, or people basing their stuff on stale code. But it isn't like this is a high churn area. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On 09/12/2010 02:42 AM, Alexander Graf wrote: On 24.08.2010, at 15:48, Alexander Graf wrote: The one big missing feature in s390-virtio was hotplugging. This is no more. This patch implements hotplug add support, so you can on the fly add new devices in the guest. Keep in mind that this needs a patch for qemu to actually leverage the functionality. Signed-off-by: Alexander Grafag...@suse.de ping (on the patch set)? Actually Marcelo applied it. But the natural place for it is Rusty's virtio tree. Rusty, if you want to take it, let me know and I'll drop it from kvm.git. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On Sun, 12 Sep 2010 06:30:43 pm Avi Kivity wrote: On 09/12/2010 02:42 AM, Alexander Graf wrote: On 24.08.2010, at 15:48, Alexander Graf wrote: The one big missing feature in s390-virtio was hotplugging. This is no more. This patch implements hotplug add support, so you can on the fly add new devices in the guest. Keep in mind that this needs a patch for qemu to actually leverage the functionality. Signed-off-by: Alexander Grafag...@suse.de ping (on the patch set)? Actually Marcelo applied it. But the natural place for it is Rusty's virtio tree. Rusty, if you want to take it, let me know and I'll drop it from kvm.git. I thought it would be in the s390 tree, which is why I didn't take it... But I'm *always* happy to let do the work! Thanks, Rusty. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On 24.08.2010, at 15:48, Alexander Graf wrote: The one big missing feature in s390-virtio was hotplugging. This is no more. This patch implements hotplug add support, so you can on the fly add new devices in the guest. Keep in mind that this needs a patch for qemu to actually leverage the functionality. Signed-off-by: Alexander Graf ag...@suse.de ping (on the patch set)? Alex -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
+static void hotplug_devices(struct work_struct *dummy)
+{
+ unsigned int i;
+ struct kvm_device_desc *d;
+ struct device *dev;
+
+ for (i = 0; i PAGE_SIZE; i += desc_size(d)) {
This should be
for (i = 0; i + desc_size(d) = PAGE_SIZE; i += desc_size(d)) {
otherwise you might have memory accesses beyond the device page...
+ d = kvm_devices + i;
+
+ /* end of list */
+ if (d-type == 0)
+ break;
...even if that should not happen if everything works.
But let's be paranoid.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On 25.08.2010, at 10:16, Heiko Carstens wrote:
On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
+static void hotplug_devices(struct work_struct *dummy)
+{
+unsigned int i;
+struct kvm_device_desc *d;
+struct device *dev;
+
+for (i = 0; i PAGE_SIZE; i += desc_size(d)) {
This should be
for (i = 0; i + desc_size(d) = PAGE_SIZE; i += desc_size(d)) {
otherwise you might have memory accesses beyond the device page...
Oh, this is a simple copypaste from the original search method. Is d valid in
the first part of the loop already?
+d = kvm_devices + i;
+
+/* end of list */
+if (d-type == 0)
+break;
...even if that should not happen if everything works.
But let's be paranoid.
Yeah :). I like paranoid.
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
On 25.08.2010, at 10:16, Heiko Carstens wrote:
On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
+static void hotplug_devices(struct work_struct *dummy)
+{
+ unsigned int i;
+ struct kvm_device_desc *d;
+ struct device *dev;
+
+ for (i = 0; i PAGE_SIZE; i += desc_size(d)) {
This should be
for (i = 0; i + desc_size(d) = PAGE_SIZE; i += desc_size(d)) {
otherwise you might have memory accesses beyond the device page...
Oh, this is a simple copypaste from the original search method.
Is d valid in the first part of the loop already?
No, you would need to initialize it with kvm_devices if you change
the loop.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On 25.08.2010, at 10:35, Heiko Carstens wrote:
On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
On 25.08.2010, at 10:16, Heiko Carstens wrote:
On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
+static void hotplug_devices(struct work_struct *dummy)
+{
+ unsigned int i;
+ struct kvm_device_desc *d;
+ struct device *dev;
+
+ for (i = 0; i PAGE_SIZE; i += desc_size(d)) {
This should be
for (i = 0; i + desc_size(d) = PAGE_SIZE; i += desc_size(d)) {
otherwise you might have memory accesses beyond the device page...
Oh, this is a simple copypaste from the original search method.
Is d valid in the first part of the loop already?
No, you would need to initialize it with kvm_devices if you change
the loop.
But even then it would take the size of the current entry, not the next one
we're actually looking for, no? Maybe it'd be easier to just convert this into
a while loop and explicitly open code everything.
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On Wed, Aug 25, 2010 at 10:34:29AM +0200, Alexander Graf wrote:
On 25.08.2010, at 10:35, Heiko Carstens wrote:
On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
On 25.08.2010, at 10:16, Heiko Carstens wrote:
On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
+static void hotplug_devices(struct work_struct *dummy)
+{
+unsigned int i;
+struct kvm_device_desc *d;
+struct device *dev;
+
+for (i = 0; i PAGE_SIZE; i += desc_size(d)) {
This should be
for (i = 0; i + desc_size(d) = PAGE_SIZE; i += desc_size(d)) {
otherwise you might have memory accesses beyond the device page...
Oh, this is a simple copypaste from the original search method.
Is d valid in the first part of the loop already?
No, you would need to initialize it with kvm_devices if you change
the loop.
But even then it would take the size of the current entry, not the next
one we're actually looking for, no? Maybe it'd be easier to just convert
this into a while loop and explicitly open code everything.
Yes indeed. It's going to be quite ugly if you want to make sure that at
no time you're going to access memory beyond the device page. Eeek.. :)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] S390: Add virtio hotplug add support
On 25.08.2010, at 10:48, Heiko Carstens wrote:
On Wed, Aug 25, 2010 at 10:34:29AM +0200, Alexander Graf wrote:
On 25.08.2010, at 10:35, Heiko Carstens wrote:
On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
On 25.08.2010, at 10:16, Heiko Carstens wrote:
On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
+static void hotplug_devices(struct work_struct *dummy)
+{
+unsigned int i;
+struct kvm_device_desc *d;
+struct device *dev;
+
+for (i = 0; i PAGE_SIZE; i += desc_size(d)) {
This should be
for (i = 0; i + desc_size(d) = PAGE_SIZE; i += desc_size(d)) {
otherwise you might have memory accesses beyond the device page...
Oh, this is a simple copypaste from the original search method.
Is d valid in the first part of the loop already?
No, you would need to initialize it with kvm_devices if you change
the loop.
But even then it would take the size of the current entry, not the next
one we're actually looking for, no? Maybe it'd be easier to just convert
this into a while loop and explicitly open code everything.
Yes indeed. It's going to be quite ugly if you want to make sure that at
no time you're going to access memory beyond the device page. Eeek.. :)
Thinking about this a bit more ... it's no problem to access beyond the device
page. After the device page follow the rings and we always have rings because
we always have a virtio console :).
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
