From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id A93A711066 for ; Sun, 25 Aug 2024 11:45:51 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id CE2D8110CB; Sun, 25 Aug 2024 11:45:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DMARC_MISSING, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fout7-smtp.messagingengine.com (fout7-smtp.messagingengine.com [103.168.172.150]) by atuin.qyliss.net (Postfix) with ESMTPS id EC304110C8 for ; Sun, 25 Aug 2024 11:45:47 +0000 (UTC) Received: from phl-compute-02.internal (phl-compute-02.nyi.internal [10.202.2.42]) by mailfout.nyi.internal (Postfix) with ESMTP id EE14A138FF95; Sun, 25 Aug 2024 07:45:46 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-02.internal (MEProxy); Sun, 25 Aug 2024 07:45:46 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1724586346; x=1724672746; bh=lcC+bAZdjp Uf1W112jUcgqZ+Tlw9BtknMvKac2ojiYg=; b=bs5M8zaGcQjOdYF3ak5wl/mDJa bZnj7SHMeNxMve0QO1JuUJj8JtEnQ483KZNh+pLgeUBtqkf88EHADxRA5EyXJcI6 tqlCfzyLFHoCLMhHfZDBOgN020h+Kvq0Q6s/haNOM3YKmHv24AG7KV9aXsGFFhiV GRdoDP206dO4iOGx08UE1seDSLq/p88pdgPG9CnwgiN3rfNvh7H8df7BqsTYM5WA VWyJoVnIilzl6yuZfsbwa80LUeOEkHRYsbiQAyOjVXvcvRskC1EOhCIEAu3qs3IO ypvbSVtTiStEP8dQfrB+Ryuvsor1OVaJeAnGiSpOkDMjCS+yXmI4B8eoK3Rg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1724586346; x=1724672746; bh=lcC+bAZdjpUf1W112jUcgqZ+Tlw9 BtknMvKac2ojiYg=; b=N/1nQ89QeibDFRS78lTXpbSNm5hVdgIiPCmWtxHEMHVq mbhauptS93HABEJKU4qRO8VCUKn8KRLnrtxt5OHw3866veZ9EXgPt4KQT+EjLRbj ze+IhsaaeWuLeb09NW3USS03nMpEL4OCdrrfr12MQ/6WqKLkyp92m4NYE0cOsciC P1Bsf0s6aqJszEjdVxtjbvaWaexOjpkGgWs/qB9qGj1KuPho4Rp2eNSMzx4ZoGTK N1vzZBWJ1MEXeo1jfUPRjm0rrIn0+fEg+77D0Zl/ThRpBd1p6fqHVmUcsxO95+bQ 9SAm+NP2SYnWKNyOOzv1yumo+mW06CH4jP4orupA7A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddviedggeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhvfevuf gjfhffkfggtgesghdtreertddtjeenucfhrhhomheptehlhihsshgrucftohhsshcuoehh ihesrghlhihsshgrrdhisheqnecuggftrfgrthhtvghrnhephefhheekheetveejjeelvd dtieeiudfgjeeutedvveelkeehkeeltdelheetkeelnecuffhomhgrihhnpehsthguohhu thdrmhgrphenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpehhihesrghlhihsshgrrdhishdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhht phhouhhtpdhrtghpthhtohepuggvvhgvlhesshhpvggtthhruhhmqdhoshdrohhrghdprh gtphhtthhopeihuhhkrgeshihukhgrrdguvghv X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 25 Aug 2024 07:45:46 -0400 (EDT) Received: by mbp.qyliss.net (Postfix, from userid 1000) id 2241257DC; Sun, 25 Aug 2024 13:45:45 +0200 (CEST) From: Alyssa Ross To: Yureka Subject: Re: [PATCH] host/start-vmm: replace jq calls with miniserde In-Reply-To: <20240825112835.207659-1-yuka@yuka.dev> References: <20240825112835.207659-1-yuka@yuka.dev> Date: Sun, 25 Aug 2024 13:45:42 +0200 Message-ID: <87o75ghlll.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Message-ID-Hash: RI2VP3BIR2HNKKAREHRKIGOG2GBI2P2Q X-Message-ID-Hash: RI2VP3BIR2HNKKAREHRKIGOG2GBI2P2Q X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: devel@spectrum-os.org X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Yureka writes: > --- > host/start-vmm/ch.rs | 38 +++++++++++++++----------------------- > 1 file changed, 15 insertions(+), 23 deletions(-) Thanks a lot for the patch! Could you please add a Signed-off-by and copyright header? (You already have one in release/combined/default.nix =E2=80=94 just needs to be like that but with the current year.) > diff --git a/host/start-vmm/ch.rs b/host/start-vmm/ch.rs > index cf164c6..abc6a00 100644 > --- a/host/start-vmm/ch.rs > +++ b/host/start-vmm/ch.rs > @@ -9,7 +9,7 @@ use std::os::raw::{c_char, c_int}; > use std::os::unix::prelude::*; > use std::process::{Command, Stdio}; >=20=20 > -use miniserde::{json, Serialize}; > +use miniserde::{json, Deserialize, Serialize}; >=20=20 > use crate::net::MacAddress; >=20=20 > @@ -126,34 +126,26 @@ pub fn create_vm(vm_name: &str, mut config: VmConfi= g) -> Result<(), String> { > } >=20=20 > pub fn add_net(vm_name: &str, net: &NetConfig) -> Result { > - let mut ch_remote =3D command(vm_name, "add-net") > + let ch_output =3D command(vm_name, "add-net") > .arg(format!("fd=3D{},mac=3D{}", net.fd, net.mac)) > - .stdout(Stdio::piped()) > .spawn() > - .or(Err(EPERM))?; > + .or(Err(EPERM))? > + .wait_with_output() > + .or(Err(EPROTO))?; >=20=20 > - let jq_out =3D match Command::new("jq") > - .args(["-j", ".id"]) > - .stdin(ch_remote.stdout.take().unwrap()) > - .stderr(Stdio::inherit()) > - .output() > - { > - Ok(o) =3D> o, > - Err(_) =3D> { > - // Try not to leave a zombie. > - let _ =3D ch_remote.kill(); > - let _ =3D ch_remote.wait(); > - return Err(EPERM); > - } > - }; > + if !ch_output.status.success() { > + return Err(EPROTO); > + } >=20=20 > - if let Ok(ch_remote_status) =3D ch_remote.wait() { > - if ch_remote_status.success() && jq_out.status.success() { > - return Ok(OsString::from_vec(jq_out.stdout)); > - } > + #[derive(Deserialize)] > + struct AddNetOutput { > + id: String, > } >=20=20 > - Err(EPROTO) > + let output_str =3D std::str::from_utf8(&ch_output.stdout).map_err(|_= | EPROTO)?; Can you use std::str at the top of the file? That's the normal convention in Spectrum. > + let output_parsed: AddNetOutput =3D json::from_str(output_str).map_e= rr(|_| EPROTO)?; > + > + Ok(OsString::from_vec(output_parsed.id.into())) Should we just change this function to return String? I think the only reason to use OsString was that there was no reason to go through a UTF-8 check before, but now miniserde is doing that anyway. This would be a bigger diff, so if you'd rather I did that myself as follow-up that's fine too. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEH9wgcxqlHM/ARR3h+dvtSFmyccAFAmbLGWYACgkQ+dvtSFmy ccDg+Q//dXpmIGkrvxOf/rWeVdr+YeuV7ym32G0+SNoFOzkrglbtkP3VfdXoO1u1 X8CGiZsqCeWDx3cDUGAjW+YojBX54SLQdpnKqiYtJrnEELnGsfe0k4yuYgqZlzrg ON/nk96DzdilOtlNFifxWHjsk8MZflTPH8OxP6RPZr3aphuclzSmB1EA+hgJn0Ya VBIxf/DfdreaUEN4a3AQ2ZRS3wkIKprw4sfVc3jBQIrqQxGe6G+4AqIz47Va/3jo sKZbX6SVbhUjZn8XlLZRMz7oFHTVPS5xaUFsuPKT4k0tyKCmLLYLqZzW8hCuvduW OLeT7+qjFutBNFpN+HT5Es6JikjZmiXTJyromlMuWxnx4dcRjOAvDHxfcmGiWc0I kdJnAbBffogK2UAXbFdzxy3DghAvK50B0JPZrxI7sQGu40OscScMIhHwrdeYvbKl UwVKrsedk87bJo5qiWr8FTQgkidyQR+0UA4Mrabt9bAtGtn5pSwkyhwpUVJjRopa 2nNvezpmdVSeLwDQ+5Rkr44sb2u6JvSqhJgjyhtzEqkvWwYQWCWXUvrDI9c04Nqh pkFrQ2izdjD31yr4zj5WzRCdvEg36xPs008ylxhugTyhwzJTzooBzQmQkZ7+6gvu 4UrlOfU9hDPKAD3AxXx2hplAWiz2aHBicrd0mRL4/FoRQzXiE4s= =IscT -----END PGP SIGNATURE----- --=-=-=--