Prevent double controlvm responses to DEVICE_CREATE, DEVICE_CHANGESTATE, and DEVICE_DESTROY

[selltc]

KanBoard-25114

After commit 87241ab85930519a962a567de58d959d3029b419 (staging-next), it looks like we are now sending double-responses for DEVICE_CREATE, DEVICE_CHANGESTATE, and DEVICE_DESTROY.

Look at the change to device_epilog():

@@ -1026,20 +965,14 @@ device_epilog(struct visor_device *dev_info,
        if (response >= 0) {
                switch (cmd) {
                case CONTROLVM_DEVICE_CREATE:
-                       if (notifiers->device_create) {
-                               (*notifiers->device_create) (dev_info);
-                               goto out_unlock;
-                       }
+                       chipset_device_create(dev_info);
                        break;
                case CONTROLVM_DEVICE_CHANGESTATE:
                        /* ServerReady / ServerRunning / SegmentStateRunning */
                        if (state.alive == segment_state_running.alive &&
                            state.operating ==
                                segment_state_running.operating) {
-                               if (notifiers->device_resume) {
-                                       (*notifiers->device_resume) (dev_info);
-                                       goto out_unlock;
-                               }
+                               chipset_device_resume(dev_info);
                        }
                        /* ServerNotReady / ServerLost / SegmentStateStandby */
                        else if (state.alive == segment_state_standby.alive &&
@@ -1049,26 +982,17 @@ device_epilog(struct visor_device *dev_info,
                                 * technically this is standby case
                                 * where server is lost
                                 */
-                               if (notifiers->device_pause) {
-                                       (*notifiers->device_pause) (dev_info);
-                                       goto out_unlock;
-                               }
+                               chipset_device_pause(dev_info);
                        }
                        break;
                case CONTROLVM_DEVICE_DESTROY:
-                       if (notifiers->device_destroy) {
-                               (*notifiers->device_destroy) (dev_info);
-                               goto out_unlock;
-                       }
+                       chipset_device_destroy(dev_info);
                        break;
                }
        }

-out_respond_and_unlock:
+out_respond:
        device_responder(cmd, pmsg_hdr, response);
-
-out_unlock:
-       up(&notifier_lock);
 }

The key point is that chipset_device_create(), chipset_device_resume(), chipset_device_pause(), and chipset_device_destroy() all have device_responder() (or device_changestate_responder(), which also signalinserts a response) calls embedded in them. Yet when we break out of the above case, we end up at out_respond, which also calls device_responder().

[davidker]

Tim, is this fixed already? I thought I address this.

[selltc]

Yeah, I thought you did too, but I will admit I haven't recently gone thru all of the github issues and updated them to reflect what has been submitted.

[davidker]

Agreed, I'm spending some time trying to clean up the Kanboard, I will look up the patch and close this one as well.

Thanks, David Kershner

Closing fixed with ca1cbf9