[PATCH v3] host/start-vmm: replace jq calls with miniserde

[PATCH v3] host/start-vmm: replace jq calls with miniserde

[Yureka]

Signed-off-by: Yureka <yuka@yuka.dev>
---
 host/start-vmm/ch.rs | 50 +++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/host/start-vmm/ch.rs b/host/start-vmm/ch.rs
index cf164c6..1ff2b26 100644
--- a/host/start-vmm/ch.rs
+++ b/host/start-vmm/ch.rs
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: EUPL-1.2+
 // SPDX-FileCopyrightText: 2022-2024 Alyssa Ross <hi@alyssa.is>
+// SPDX-FileCopyrightText: 2024 Yureka <yuka@yuka.dev>
 
 use std::ffi::{CStr, OsStr, OsString};
 use std::io::Write;
@@ -8,8 +9,9 @@ use std::num::NonZeroI32;
 use std::os::raw::{c_char, c_int};
 use std::os::unix::prelude::*;
 use std::process::{Command, Stdio};
+use std::str;
 
-use miniserde::{json, Serialize};
+use miniserde::{json, Deserialize, Serialize};
 
 use crate::net::MacAddress;
 
@@ -125,38 +127,30 @@ pub fn create_vm(vm_name: &str, mut config: VmConfig) -> Result<(), String> {
     Ok(())
 }
 
-pub fn add_net(vm_name: &str, net: &NetConfig) -> Result<OsString, NonZeroI32> {
-    let mut ch_remote = command(vm_name, "add-net")
+pub fn add_net(vm_name: &str, net: &NetConfig) -> Result<String, NonZeroI32> {
+    let ch_output = command(vm_name, "add-net")
         .arg(format!("fd={},mac={}", net.fd, net.mac))
-        .stdout(Stdio::piped())
         .spawn()
-        .or(Err(EPERM))?;
+        .or(Err(EPERM))?
+        .wait_with_output()
+        .or(Err(EPROTO))?;
 
-    let jq_out = match Command::new("jq")
-        .args(["-j", ".id"])
-        .stdin(ch_remote.stdout.take().unwrap())
-        .stderr(Stdio::inherit())
-        .output()
-    {
-        Ok(o) => o,
-        Err(_) => {
-            // Try not to leave a zombie.
-            let _ = ch_remote.kill();
-            let _ = ch_remote.wait();
-            return Err(EPERM);
-        }
-    };
+    if !ch_output.status.success() {
+        return Err(EPROTO);
+    }
 
-    if let Ok(ch_remote_status) = 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,
     }
 
-    Err(EPROTO)
+    let output_str = str::from_utf8(&ch_output.stdout).map_err(|_| EPROTO)?;
+    let output_parsed: AddNetOutput = json::from_str(output_str).map_err(|_| EPROTO)?;
+
+    Ok(output_parsed.id)
 }
 
-pub fn remove_device(vm_name: &str, device_id: &OsStr) -> Result<(), NonZeroI32> {
+pub fn remove_device(vm_name: &str, device_id: &str) -> Result<(), NonZeroI32> {
     let ch_remote = command(vm_name, "remove-device")
         .arg(device_id)
         .status()
@@ -178,7 +172,7 @@ pub fn remove_device(vm_name: &str, device_id: &OsStr) -> Result<(), NonZeroI32>
 unsafe extern "C" fn add_net_c(
     vm_name: *const c_char,
     net: &NetConfig,
-    id: *mut *mut OsString,
+    id: *mut *mut String,
 ) -> c_int {
     let Ok(vm_name) = CStr::from_ptr(vm_name).to_str() else {
         return EINVAL.into();
@@ -202,7 +196,7 @@ unsafe extern "C" fn add_net_c(
 /// - `id` must be a device ID obtained by calling `add_net_c`.  After
 ///   calling `remove_device_c`, the pointer is no longer valid.
 #[export_name = "ch_remove_device"]
-unsafe extern "C" fn remove_device_c(vm_name: *const c_char, device_id: *mut OsString) -> c_int {
+unsafe extern "C" fn remove_device_c(vm_name: *const c_char, device_id: *mut String) -> c_int {
     let Ok(vm_name) = CStr::from_ptr(vm_name).to_str() else {
         return EINVAL.into();
     };
@@ -220,7 +214,7 @@ unsafe extern "C" fn remove_device_c(vm_name: *const c_char, device_id: *mut OsS
 /// `id` must be a device ID obtained by calling `add_net_c`.  After
 /// calling `device_free`, the pointer is no longer valid.
 #[export_name = "ch_device_free"]
-unsafe extern "C" fn device_free(id: *mut OsString) {
+unsafe extern "C" fn device_free(id: *mut String) {
     if !id.is_null() {
         drop(Box::from_raw(id))
     }
-- 
2.45.2

Re: [PATCH v3] host/start-vmm: replace jq calls with miniserde

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

On Sun, Aug 25, 2024 at 08:27:24PM GMT, Yureka wrote:
> Signed-off-by: Yureka <yuka@yuka.dev>
> ---
>  host/start-vmm/ch.rs | 50 +++++++++++++++++++-------------------------
>  1 file changed, 22 insertions(+), 28 deletions(-)

Hi!  This looked right to me, but when I test it by running "make run"
in host/rootfs, it doesn't work:

run: setting up networking failed: Protocol error (os error 71)

Want to look into it?  I'm happy to if you'd rather not. :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] host/start-vmm: replace jq calls with miniserde

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On Mon, Aug 26, 2024 at 12:01:22PM GMT, Alyssa Ross wrote:
> On Sun, Aug 25, 2024 at 08:27:24PM GMT, Yureka wrote:
> > Signed-off-by: Yureka <yuka@yuka.dev>
> > ---
> >  host/start-vmm/ch.rs | 50 +++++++++++++++++++-------------------------
> >  1 file changed, 22 insertions(+), 28 deletions(-)
>
> Hi!  This looked right to me, but when I test it by running "make run"
> in host/rootfs, it doesn't work:
>
> run: setting up networking failed: Protocol error (os error 71)

I wouldn't mind some eprintln!()s to log what the error that got turned
into EPROTO was either…

> Want to look into it?  I'm happy to if you'd rather not. :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]