Waiting event of vsock running before use it when use_vsock enabled #1918

BetaXOi · 2019-07-29T08:48:31Z

Fixes: #1917

kata-runtime needs wait qemu report event of vsock is running.
see BetaXOi/qemu@9b536b6

Signed-off-by: Ning Bo n.b@live.com

grahamwhaley · 2019-07-29T08:57:48Z

Nice find @BetaXOi - pulling in some folks to review...

Gopkg.toml

virtcontainers/qemu.go

Gopkg.lock

BetaXOi · 2019-07-29T14:34:46Z

Hello everyone, this PR is a simple methed to fix #1917, it's a demo just.
I think we should wait the 'report vsock event' has been merged in Qemu first, then we talk about method of QMP version check next.

devimc · 2019-07-29T14:36:46Z

thanks @BetaXOi I'll mark this as WIP and DNM

BetaXOi · 2019-07-31T12:46:40Z

@jodh-intel @markdryan I find a new way to fix it without check Qemu version, but need modify govmm, please help review.

jodh-intel

Hi @BetaXOi - thanks for raising, but the govmm change needs to be raised as a PR for https://github.com/intel/govmm.

vendor/github.com/intel/govmm/qemu/qmp.go

amshinde · 2019-07-31T19:22:04Z

@stefanha This fell off my radar for a bit. Can you take a look at this PR?

Report vsock running event so that the upper application can control boot sequence. see kata-containers/runtime#1918 Signed-off-by: Ning Bo <ning.bo9@zte.com.cn>

The upper hyervisor manager application maybe need to wait some QMP event to control boot sequence, but the event we wanted maybe not exist in some older version, so we need query all QMP ABI and check the event is supported or not. related: kata-containers/runtime#1918 Signed-off-by: Ning Bo <ning.bo9@zte.com.cn>

vendor/github.com/intel/govmm/qemu/qmp.go

The upper hyervisor manager application maybe need to wait some QMP event to control boot sequence, but the event we wanted maybe not exist in some older version, so we need query all QMP ABI and check the event is supported or not. related: kata-containers/runtime#1918 Signed-off-by: Ning Bo <ning.bo9@zte.com.cn>

BetaXOi · 2019-08-02T02:21:02Z

I have updated the commit with revendor govmm, please help review

jodh-intel · 2019-08-02T07:53:31Z

Thanks @BetaXOi. Travis is failing and if you look at the log:

Found 1 commit between commit bdbecabafd419bdb6a992502553e45c9cc10f4bf and branch master
ERROR: Commit bdbecabafd419bdb6a992502553e45c9cc10f4bf: Failed to find subsystem in subject: "Waiting event of vsock running before use it when use_vsock enabled"
ERROR: checkcommits failed. See the document below for help on formatting
commits for the project.

https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#patch-format

https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#patch-format gives a full explanation and examples but in your case, I'd change your commit message to the following to keep Travis happy:

vsock: Waiting event of vsock running before use it when use_vsock enabled

Wait for qemu to report that vsock is running. See BetaXOi/qemu@9b536b6.

Fixes: #1917.

Signed-off-by: Ning Bo <ning.bo9@zte.com.cn>

amshinde · 2019-08-02T17:05:11Z

virtcontainers/qemu.go

+
+	q.Logger().Info("waiting event of vsock running")
+	for {
+		ev := <-eventCh


@ningbo9 @BetaXOi Is there a possibility of a race here ie you may have missed the vosck "running" event while you are querying for the schema? If so, I would suggest listening on the channel in a separate go-routine that is started as early as possible.

In the current PR, query-qmp-schema is almost the first QMP command (because qmp_capabilities must be executed before any other QMP commands are executed) and is called immediately after LaunchQemu, and eventCh receives the event until we read it.

I have no idea how to get this process earlier.

…abled If `kata-runtime` connect vsock device in guest but the device is not start ready, the 'connect' will block and timeout after 2 sencond. This will cause the boot is slower 2 second when use vsock than serial. So we should wait 'VSOCK_RUNNING' QMP event before connect to avoid this case. Fixes: kata-containers#1917 Signed-off-by: Ning Bo <ning.bo9@zte.com.cn>

BetaXOi · 2019-08-05T06:22:06Z

@devimc I have updated the commit as suggested, the PR is ready to be merge, could you remove the 'wip' label and trigger the CI?

grahamwhaley · 2019-08-05T09:52:48Z

/test

devimc · 2019-08-05T13:17:45Z

virtcontainers/qemu.go

+		EventCh: eventCh,
+		Logger:  newQMPLogger(),
+		// response of qmp.ExecuteQMPCapabilities bigger than default 64k in bufio.NewScanner()
+		MaxCapacity: 512 * 1024,


can this be a constant ?

In most cases, we don't need to modify this value. We only need to keep the default value. In special cases, we need to modify the value according to the actual situation, but we can't find a suitable value to adapt to any situation.

I mean, can 512 * 1024 be a constant and use it here
for example:

MaxCapacity: QMPCapabilitiesMaxCap,

I see, the benefit of doing this is that it is convenient to do unit testing, right?
So, do I need to add an additional test case?

yes, defining constant is useful for unit testing, not sure if it's possible to add unit tests for this function, but if you can, go for it please

devimc · 2019-08-05T13:19:23Z

virtcontainers/qemu.go

+				break Loop
+			}
+		case <-time.After(time.Duration(timeout)*time.Second - time.Since(timeStart)):
+			q.Logger().Info("waiting event of vsock running")


I think this log is not needed

I will remove it in next commit

or you can move it before for

amshinde · 2019-08-06T22:18:48Z

Hi @BetaXOi, if I understand correctly, this patch depends on a qemu patch to send the qmp event when vsock is running.
Is the patch merged in the qemu upstream?

@ningbo9 @BetaXOi Is there a possibility of a race here ie you may have missed the vosck "running" event while you are querying for the schema? If so, I would suggest listening on the channel in a separate go-routine that is started as early as possible.

Also, can you address the above comment? This seems to have been marked as resolved.

BetaXOi · 2019-08-07T02:26:22Z

Hi @BetaXOi, if I understand correctly, this patch depends on a qemu patch to send the qmp event when vsock is running.

Yes, if qemu does not support reporting the VSOCK_RUNNING event, kata will not wait for the event, the entire process rolls back to the original state, as I described in #1917.

Is the patch merged in the qemu upstream?

No, the patch has not been merged yet
https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg00492.html

stefanha · 2019-08-09T13:37:12Z

@amshinde Thanks for letting me know. I'll discuss this upstream in QEMU.

devimc · 2019-08-09T13:42:34Z

@stefanha https://patchew.org/QEMU/1564975951-29263-1-git-send-email-ning.bo9@zte.com.cn/

amshinde · 2019-08-14T20:49:04Z

@BetaXOi Since there is discussion on the qemu patch on the right way to fix this, lets wait on qemu patch being merged first.
@BetaXOi Let us know if you plan to look at the vhost_vsock.ko problem that @stefanha has pointed out.
Marking this as "do-not-merge" till then.

egernst · 2019-09-12T20:30:08Z

Any updates here @amshinde @stefanha ?

stefanha · 2019-09-16T15:11:45Z

@BetaXOi Do you want to respond to my qemu-devel mailing list reply so we can make progress on this issue? https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01672.html

raravena80 · 2019-11-15T19:53:41Z

@BetaXOi @stefanha any updates on this PR? Thx!

stefanha · 2019-11-16T08:09:14Z

@raravena80 I haven't seen a response from @BetaXOi:
https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01672.html

raravena80 · 2019-12-20T17:42:51Z

@BetaXOi ping.

Your weekly Kata herder.

BetaXOi · 2019-12-21T08:03:08Z

After some discussion through the mail, it may be possible to solve the problem with a reverse connection. Close this PR first, I will submit a new one.

FYI: https://lists.nongnu.org/archive/html/qemu-devel/2019-12/msg04233.html

grahamwhaley requested review from devimc and gnawux July 29, 2019 08:57

jodh-intel reviewed Jul 29, 2019

View reviewed changes

Gopkg.toml Outdated Show resolved Hide resolved

virtcontainers/qemu.go Outdated Show resolved Hide resolved

devimc reviewed Jul 29, 2019

View reviewed changes

Gopkg.lock Outdated Show resolved Hide resolved

devimc added do-not-merge PR has problems or depends on another wip Work in Progress (PR incomplete - needs more work or rework) labels Jul 29, 2019

BetaXOi force-pushed the vsock-event branch from 9f38913 to 26ce474 Compare July 31, 2019 12:40

jodh-intel suggested changes Jul 31, 2019

View reviewed changes

vendor/github.com/intel/govmm/qemu/qmp.go Show resolved Hide resolved

vendor/github.com/intel/govmm/qemu/qmp.go Show resolved Hide resolved

BetaXOi mentioned this pull request Aug 1, 2019

qmp: support command 'query-qmp-schema' kata-containers/govmm#104

Merged

jodh-intel suggested changes Aug 1, 2019

View reviewed changes

vendor/github.com/intel/govmm/qemu/qmp.go Show resolved Hide resolved

BetaXOi force-pushed the vsock-event branch from 26ce474 to bdbecab Compare August 2, 2019 02:15

BetaXOi force-pushed the vsock-event branch from bdbecab to 82841e1 Compare August 2, 2019 15:26

amshinde reviewed Aug 2, 2019

View reviewed changes

BetaXOi force-pushed the vsock-event branch from 82841e1 to 32a233d Compare August 3, 2019 11:50

grahamwhaley removed the do-not-merge PR has problems or depends on another label Aug 5, 2019

grahamwhaley removed the wip Work in Progress (PR incomplete - needs more work or rework) label Aug 5, 2019

devimc reviewed Aug 5, 2019

View reviewed changes

amshinde added the do-not-merge PR has problems or depends on another label Aug 14, 2019

ahsan518 mentioned this pull request Sep 12, 2019

vsock everywhere: Clearlinux supports vsock. Use vsock for QEMU and Firecracker clearlinux/cloud-native-setup#56

Open

BetaXOi closed this Dec 21, 2019

Pennyzct mentioned this pull request Apr 28, 2020

Sandbox creating is slower when use_vsock enabled #1917

Closed

Waiting event of vsock running before use it when use_vsock enabled #1918

Waiting event of vsock running before use it when use_vsock enabled #1918

BetaXOi commented Jul 29, 2019

grahamwhaley commented Jul 29, 2019

BetaXOi commented Jul 29, 2019

devimc commented Jul 29, 2019

BetaXOi commented Jul 31, 2019

jodh-intel left a comment

amshinde commented Jul 31, 2019

BetaXOi commented Aug 2, 2019

jodh-intel commented Aug 2, 2019

amshinde Aug 2, 2019 •

edited

BetaXOi Aug 7, 2019

BetaXOi commented Aug 5, 2019

grahamwhaley commented Aug 5, 2019

devimc Aug 5, 2019

BetaXOi Aug 5, 2019

devimc Aug 5, 2019

BetaXOi Aug 5, 2019

devimc Aug 5, 2019

devimc Aug 5, 2019

BetaXOi Aug 5, 2019

devimc Aug 5, 2019

amshinde commented Aug 6, 2019

BetaXOi commented Aug 7, 2019 •

edited

stefanha commented Aug 9, 2019

devimc commented Aug 9, 2019

amshinde commented Aug 14, 2019

egernst commented Sep 12, 2019

stefanha commented Sep 16, 2019

raravena80 commented Nov 15, 2019

stefanha commented Nov 16, 2019

raravena80 commented Dec 20, 2019

BetaXOi commented Dec 21, 2019

Waiting event of vsock running before use it when use_vsock enabled #1918

Waiting event of vsock running before use it when use_vsock enabled #1918

Conversation

BetaXOi commented Jul 29, 2019

grahamwhaley commented Jul 29, 2019

BetaXOi commented Jul 29, 2019

devimc commented Jul 29, 2019

BetaXOi commented Jul 31, 2019

jodh-intel left a comment

Choose a reason for hiding this comment

amshinde commented Jul 31, 2019

BetaXOi commented Aug 2, 2019

jodh-intel commented Aug 2, 2019

amshinde Aug 2, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BetaXOi commented Aug 5, 2019

grahamwhaley commented Aug 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amshinde commented Aug 6, 2019

BetaXOi commented Aug 7, 2019 • edited

stefanha commented Aug 9, 2019

devimc commented Aug 9, 2019

amshinde commented Aug 14, 2019

egernst commented Sep 12, 2019

stefanha commented Sep 16, 2019

raravena80 commented Nov 15, 2019

stefanha commented Nov 16, 2019

raravena80 commented Dec 20, 2019

BetaXOi commented Dec 21, 2019

amshinde Aug 2, 2019 •

edited

BetaXOi commented Aug 7, 2019 •

edited