Merge pull request #2409 from sameo/topic/agent

agent: Fix cargo 1.54 clippy warning

src/agent/rustjail/src/cgroups/fs/mod.rs

@@ -232,19 +232,19 @@ fn set_devices_resources(
     let mut devices = vec![];
 
     for d in device_resources.iter() {
-        if let Some(dev) = linux_device_group_to_cgroup_device(&d) {
+        if let Some(dev) = linux_device_group_to_cgroup_device(d) {
             devices.push(dev);
         }
     }
 
     for d in DEFAULT_DEVICES.iter() {
-        if let Some(dev) = linux_device_to_cgroup_device(&d) {
+        if let Some(dev) = linux_device_to_cgroup_device(d) {
             devices.push(dev);
         }
     }
 
     for d in DEFAULT_ALLOWED_DEVICES.iter() {
-        if let Some(dev) = linux_device_group_to_cgroup_device(&d) {
+        if let Some(dev) = linux_device_group_to_cgroup_device(d) {
             devices.push(dev);
         }
     }
@@ -828,7 +828,7 @@ fn get_blkio_stats_v2(cg: &cgroups::Cgroup) -> SingularPtrField<BlkioStats> {
 
 fn get_blkio_stats(cg: &cgroups::Cgroup) -> SingularPtrField<BlkioStats> {
     if cg.v2() {
-        return get_blkio_stats_v2(&cg);
+        return get_blkio_stats_v2(cg);
     }
 
     let blkio_controller: &BlkIoController = get_controller_or_return_singular_none!(cg);
@@ -1022,7 +1022,7 @@ impl Manager {
                 .unwrap()
                 .trim_start_matches(root_path.to_str().unwrap());
             info!(sl!(), "updating cpuset for parent path {:?}", &r_path);
-            let cg = new_cgroup(cgroups::hierarchies::auto(), &r_path);
+            let cg = new_cgroup(cgroups::hierarchies::auto(), r_path);
             let cpuset_controller: &CpuSetController = cg.controller_of().unwrap();
             cpuset_controller.set_cpus(guest_cpuset)?;
         }

src/agent/rustjail/src/container.rs

@@ -390,7 +390,7 @@ fn do_init_child(cwfd: RawFd) -> Result<()> {
     let linux = spec.linux.as_ref().unwrap();
 
     // get namespace vector to join/new
-    let nses = get_namespaces(&linux);
+    let nses = get_namespaces(linux);
 
     let mut userns = false;
     let mut to_new = CloneFlags::empty();
@@ -939,7 +939,7 @@ impl BaseContainer for LinuxContainer {
 
         join_namespaces(
             &logger,
-            &spec,
+            spec,
             &p,
             self.cgroup_manager.as_ref().unwrap(),
             &st,
@@ -1031,7 +1031,7 @@ impl BaseContainer for LinuxContainer {
         let fifo = format!("{}/{}", &self.root, EXEC_FIFO_FILENAME);
         let fd = fcntl::open(fifo.as_str(), OFlag::O_WRONLY, Mode::from_bits_truncate(0))?;
         let data: &[u8] = &[0];
-        unistd::write(fd, &data)?;
+        unistd::write(fd, data)?;
         info!(self.logger, "container started");
         self.init_process_start_time = SystemTime::now()
             .duration_since(SystemTime::UNIX_EPOCH)

src/agent/rustjail/src/mount.rs

@@ -189,7 +189,7 @@ pub fn init_rootfs(
 
     let mut bind_mount_dev = false;
     for m in &spec.mounts {
-        let (mut flags, pgflags, data) = parse_mount(&m);
+        let (mut flags, pgflags, data) = parse_mount(m);
         if !m.destination.starts_with('/') || m.destination.contains("..") {
             return Err(anyhow!(
                 "the mount destination {} is invalid",
@@ -198,7 +198,7 @@ pub fn init_rootfs(
         }
 
         if m.r#type == "cgroup" {
-            mount_cgroups(cfd_log, &m, rootfs, flags, &data, cpath, mounts)?;
+            mount_cgroups(cfd_log, m, rootfs, flags, &data, cpath, mounts)?;
         } else {
             if m.destination == "/dev" {
                 if m.r#type == "bind" {
@@ -226,7 +226,7 @@ pub fn init_rootfs(
                 }
             }
 
-            mount_from(cfd_log, &m, &rootfs, flags, &data, "")?;
+            mount_from(cfd_log, m, rootfs, flags, &data, "")?;
             // bind mount won't change mount options, we need remount to make mount options
             // effective.
             // first check that we have non-default options required before attempting a
@@ -356,7 +356,7 @@ fn mount_cgroups(
     mounts: &HashMap<String, String>,
 ) -> Result<()> {
     if cgroups::hierarchies::is_cgroup2_unified_mode() {
-        return mount_cgroups_v2(cfd_log, &m, rootfs, flags);
+        return mount_cgroups_v2(cfd_log, m, rootfs, flags);
     }
     // mount tmpfs
     let ctm = Mount {

src/agent/rustjail/src/validator.rs

@@ -266,7 +266,7 @@ pub fn validate(conf: &Config) -> Result<()> {
     security(oci).context("security")?;
     usernamespace(oci).context("usernamespace")?;
     cgroupnamespace(oci).context("cgroupnamespace")?;
-    sysctl(&oci).context("sysctl")?;
+    sysctl(oci).context("sysctl")?;
 
     if conf.rootless_euid {
         rootless_euid(oci).context("rootless euid")?;

src/agent/src/config.rs

@@ -372,8 +372,8 @@ mod tests {
     #[test]
     fn test_new() {
         let config = AgentConfig::new();
-        assert_eq!(config.debug_console, false);
+        assert!(!config.debug_console);
-        assert_eq!(config.dev_mode, false);
+        assert!(!config.dev_mode);
         assert_eq!(config.log_level, DEFAULT_LOG_LEVEL);
         assert_eq!(config.hotplug_timeout, DEFAULT_HOTPLUG_TIMEOUT);
     }
@@ -754,9 +754,9 @@ mod tests {
             }
 
             let mut config = AgentConfig::new();
-            assert_eq!(config.debug_console, false, "{}", msg);
+            assert!(!config.debug_console, "{}", msg);
-            assert_eq!(config.dev_mode, false, "{}", msg);
+            assert!(!config.dev_mode, "{}", msg);
-            assert_eq!(config.unified_cgroup_hierarchy, false, "{}", msg);
+            assert!(!config.unified_cgroup_hierarchy, "{}", msg);
             assert_eq!(
                 config.hotplug_timeout,
                 time::Duration::from_secs(3),

src/agent/src/device.rs

@@ -966,12 +966,12 @@ mod tests {
         uev_a.subsystem = "block".to_string();
         uev_a.devname = devname.to_string();
         uev_a.devpath = format!("{}{}/virtio4/block/{}", root_bus, relpath_a, devname);
-        let matcher_a = VirtioBlkPciMatcher::new(&relpath_a);
+        let matcher_a = VirtioBlkPciMatcher::new(relpath_a);
 
         let mut uev_b = uev_a.clone();
         let relpath_b = "/0000:00:0a.0/0000:00:0b.0";
         uev_b.devpath = format!("{}{}/virtio0/block/{}", root_bus, relpath_b, devname);
-        let matcher_b = VirtioBlkPciMatcher::new(&relpath_b);
+        let matcher_b = VirtioBlkPciMatcher::new(relpath_b);
 
         assert!(matcher_a.is_match(&uev_a));
         assert!(matcher_b.is_match(&uev_b));
@@ -1053,7 +1053,7 @@ mod tests {
             "{}/0000:00:00.0/virtio0/host0/target0:0:0/0:0:{}/block/sda",
             root_bus, addr_a
         );
-        let matcher_a = ScsiBlockMatcher::new(&addr_a);
+        let matcher_a = ScsiBlockMatcher::new(addr_a);
 
         let mut uev_b = uev_a.clone();
         let addr_b = "2:0";
@@ -1061,7 +1061,7 @@ mod tests {
             "{}/0000:00:00.0/virtio0/host0/target0:0:2/0:0:{}/block/sdb",
             root_bus, addr_b
         );
-        let matcher_b = ScsiBlockMatcher::new(&addr_b);
+        let matcher_b = ScsiBlockMatcher::new(addr_b);
 
         assert!(matcher_a.is_match(&uev_a));
         assert!(matcher_b.is_match(&uev_b));

src/agent/src/main.rs

@@ -302,7 +302,7 @@ async fn start_sandbox(
     }
 
     // Initialize unique sandbox structure.
-    let s = Sandbox::new(&logger).context("Failed to create sandbox")?;
+    let s = Sandbox::new(logger).context("Failed to create sandbox")?;
     if init_mode {
         s.rtnl.handle_localhost().await?;
     }

src/agent/src/metrics.rs

@@ -193,7 +193,7 @@ fn update_guest_metrics() {
         Ok(kernel_stats) => {
             set_gauge_vec_cpu_time(&GUEST_CPU_TIME, "total", &kernel_stats.total);
             for (i, cpu_time) in kernel_stats.cpu_time.iter().enumerate() {
-                set_gauge_vec_cpu_time(&GUEST_CPU_TIME, format!("{}", i).as_str(), &cpu_time);
+                set_gauge_vec_cpu_time(&GUEST_CPU_TIME, format!("{}", i).as_str(), cpu_time);
             }
         }
     }

src/agent/src/mount.rs

@@ -282,7 +282,7 @@ async fn ephemeral_storage_handler(
             fs::set_permissions(&storage.mount_point, permission)?;
         }
     } else {
-        common_storage_handler(logger, &storage)?;
+        common_storage_handler(logger, storage)?;
     }
 
     Ok("".to_string())
@@ -1104,8 +1104,8 @@ mod tests {
 
         // Create an actual mount
         let bare_mount = BareMount::new(
-            &mnt_src_filename,
+            mnt_src_filename,
-            &mnt_dest_filename,
+            mnt_dest_filename,
             "bind",
             MsFlags::MS_BIND,
             "",
@@ -1274,7 +1274,7 @@ mod tests {
         let logger = slog::Logger::root(drain, o!());
         let result = get_cgroup_mounts(&logger, "", true);
 
-        assert_eq!(true, result.is_ok());
+        assert!(result.is_ok());
         let result = result.unwrap();
         assert_eq!(1, result.len());
         assert_eq!(result[0].fstype, "cgroup2");

src/agent/src/namespace.rs

@@ -102,7 +102,7 @@ impl Namespace {
 
         let new_thread = tokio::spawn(async move {
             if let Err(err) = || -> Result<()> {
-                let origin_ns_path = get_current_thread_ns_path(&ns_type.get());
+                let origin_ns_path = get_current_thread_ns_path(ns_type.get());
 
                 File::open(Path::new(&origin_ns_path))?;
 

src/agent/src/netlink.rs

@@ -82,8 +82,8 @@ impl Handle {
 
         // Add new ip addresses from request
         for ip_address in &iface.IPAddresses {
-            let ip = IpAddr::from_str(&ip_address.get_address())?;
+            let ip = IpAddr::from_str(ip_address.get_address())?;
-            let mask = u8::from_str_radix(ip_address.get_mask(), 10)?;
+            let mask = ip_address.get_mask().parse::<u8>()?;
 
             self.add_addresses(link.index(), std::iter::once(IpNetwork::new(ip, mask)?))
                 .await?;
@@ -512,7 +512,7 @@ impl Handle {
             .and_then(|addr| if addr.is_empty() { None } else { Some(addr) }) // Make sure it's not empty
             .ok_or(nix::Error::Sys(nix::errno::Errno::EINVAL))?;
 
-        let ip = IpAddr::from_str(&ip_address)
+        let ip = IpAddr::from_str(ip_address)
             .map_err(|e| anyhow!("Failed to parse IP {}: {:?}", ip_address, e))?;
 
         // Import rtnetlink objects that make sense only for this function

src/agent/src/network.rs

@@ -127,16 +127,11 @@ mod tests {
         // call do_setup_guest_dns
         let result = do_setup_guest_dns(logger, dns.clone(), src_filename, dst_filename);
 
-        assert_eq!(
+        assert!(result.is_ok(), "result should be ok, but {:?}", result);
-            true,
-            result.is_ok(),
-            "result should be ok, but {:?}",
-            result
-        );
 
         // get content of /etc/resolv.conf
         let content = fs::read_to_string(dst_filename);
-        assert_eq!(true, content.is_ok());
+        assert!(content.is_ok());
         let content = content.unwrap();
 
         let expected_dns: Vec<&str> = content.split('\n').collect();

src/agent/src/rpc.rs

@@ -190,7 +190,7 @@ impl AgentService {
         let p = if oci.process.is_some() {
             Process::new(
                 &sl!(),
-                &oci.process.as_ref().unwrap(),
+                oci.process.as_ref().unwrap(),
                 cid.as_str(),
                 true,
                 pipe_size,
@@ -247,7 +247,7 @@ impl AgentService {
             // Find the sandbox storage used by this container
             let mounts = sandbox.container_mounts.get(&cid);
             if let Some(mounts) = mounts {
-                remove_mounts(&mounts)?;
+                remove_mounts(mounts)?;
 
                 for m in mounts.iter() {
                     if sandbox.storages.get(m).is_some() {
@@ -666,7 +666,7 @@ impl protocols::agent_ttrpc::AgentService for AgentService {
         let s = Arc::clone(&self.sandbox);
         let mut sandbox = s.lock().await;
 
-        let ctr = sandbox.get_container(&cid).ok_or_else(|| {
+        let ctr = sandbox.get_container(cid).ok_or_else(|| {
             ttrpc_error(
                 ttrpc::Code::INVALID_ARGUMENT,
                 "invalid container id".to_string(),
@@ -689,7 +689,7 @@ impl protocols::agent_ttrpc::AgentService for AgentService {
         let s = Arc::clone(&self.sandbox);
         let mut sandbox = s.lock().await;
 
-        let ctr = sandbox.get_container(&cid).ok_or_else(|| {
+        let ctr = sandbox.get_container(cid).ok_or_else(|| {
             ttrpc_error(
                 ttrpc::Code::INVALID_ARGUMENT,
                 "invalid container id".to_string(),

src/agent/src/sandbox.rs

@@ -272,7 +272,7 @@ impl Sandbox {
             ctr.cgroup_manager
                 .as_ref()
                 .unwrap()
-                .update_cpuset_path(guest_cpuset.as_str(), &container_cpust)?;
+                .update_cpuset_path(guest_cpuset.as_str(), container_cpust)?;
         }
 
         Ok(())
@@ -461,7 +461,7 @@ mod tests {
     use tempfile::Builder;
 
     fn bind_mount(src: &str, dst: &str, logger: &Logger) -> Result<(), Error> {
-        let baremount = BareMount::new(src, dst, "bind", MsFlags::MS_BIND, "", &logger);
+        let baremount = BareMount::new(src, dst, "bind", MsFlags::MS_BIND, "", logger);
         baremount.mount()
     }
 
@@ -474,7 +474,7 @@ mod tests {
         let tmpdir_path = tmpdir.path().to_str().unwrap();
 
         // Add a new sandbox storage
-        let new_storage = s.set_sandbox_storage(&tmpdir_path);
+        let new_storage = s.set_sandbox_storage(tmpdir_path);
 
         // Check the reference counter
         let ref_count = s.storages[tmpdir_path];
@@ -483,11 +483,11 @@ mod tests {
             "Invalid refcount, got {} expected 1.",
             ref_count
         );
-        assert_eq!(new_storage, true);
+        assert!(new_storage);
 
         // Use the existing sandbox storage
-        let new_storage = s.set_sandbox_storage(&tmpdir_path);
+        let new_storage = s.set_sandbox_storage(tmpdir_path);
-        assert_eq!(new_storage, false, "Should be false as already exists.");
+        assert!(!new_storage, "Should be false as already exists.");
 
         // Since we are using existing storage, the reference counter
         // should be 2 by now.
@@ -527,7 +527,7 @@ mod tests {
             .unwrap();
 
         assert!(
-            s.remove_sandbox_storage(&srcdir_path).is_err(),
+            s.remove_sandbox_storage(srcdir_path).is_err(),
             "Expect Err as the directory i not a mountpoint"
         );
 
@@ -586,8 +586,8 @@ mod tests {
 
         assert!(bind_mount(srcdir_path, destdir_path, &logger).is_ok());
 
-        assert_eq!(s.set_sandbox_storage(&destdir_path), true);
+        assert!(s.set_sandbox_storage(destdir_path));
-        assert!(s.unset_and_remove_sandbox_storage(&destdir_path).is_ok());
+        assert!(s.unset_and_remove_sandbox_storage(destdir_path).is_ok());
 
         let other_dir_str;
         {
@@ -600,7 +600,7 @@ mod tests {
             let other_dir_path = other_dir.path().to_str().unwrap();
             other_dir_str = other_dir_path.to_string();
 
-            assert_eq!(s.set_sandbox_storage(&other_dir_path), true);
+            assert!(s.set_sandbox_storage(other_dir_path));
         }
 
         assert!(s.unset_and_remove_sandbox_storage(&other_dir_str).is_err());
@@ -614,17 +614,15 @@ mod tests {
         let storage_path = "/tmp/testEphe";
 
         // Add a new sandbox storage
-        assert_eq!(s.set_sandbox_storage(&storage_path), true);
+        assert!(s.set_sandbox_storage(storage_path));
         // Use the existing sandbox storage
-        assert_eq!(
+        assert!(
-            s.set_sandbox_storage(&storage_path),
+            !s.set_sandbox_storage(storage_path),
-            false,
             "Expects false as the storage is not new."
         );
 
-        assert_eq!(
+        assert!(
-            s.unset_sandbox_storage(&storage_path).unwrap(),
+            !s.unset_sandbox_storage(storage_path).unwrap(),
-            false,
             "Expects false as there is still a storage."
         );
 
@@ -636,9 +634,8 @@ mod tests {
             ref_count
         );
 
-        assert_eq!(
+        assert!(
-            s.unset_sandbox_storage(&storage_path).unwrap(),
+            s.unset_sandbox_storage(storage_path).unwrap(),
-            true,
             "Expects true as there is still a storage."
         );
 
@@ -654,7 +651,7 @@ mod tests {
         // If no container is using the sandbox storage, the reference
         // counter for it should not exist.
         assert!(
-            s.unset_sandbox_storage(&storage_path).is_err(),
+            s.unset_sandbox_storage(storage_path).is_err(),
             "Expects false as the reference counter should no exist."
         );
     }

src/agent/src/uevent.rs

@@ -87,14 +87,14 @@ impl Uevent {
         sb.uevent_map.insert(self.devpath.clone(), self.clone());
 
         // Notify watchers that are interested in the udev event.
-        for watch in &mut sb.uevent_watchers {
+        sb.uevent_watchers.iter_mut().for_each(move |watch| {
             if let Some((matcher, _)) = watch {
-                if matcher.is_match(&self) {
+                if matcher.is_match(self) {
                     let (_, sender) = watch.take().unwrap();
                     let _ = sender.send(self.clone());
                 }
             }
-        }
+        })
     }
 
     #[instrument]
@@ -221,15 +221,17 @@ pub(crate) fn spawn_test_watcher(sandbox: Arc<Mutex<Sandbox>>, uev: Uevent) {
     tokio::spawn(async move {
         loop {
             let mut sb = sandbox.lock().await;
-            for w in &mut sb.uevent_watchers {
+            let uev = uev.clone();
-                if let Some((matcher, _)) = w {
+            sb.uevent_watchers.iter_mut().for_each(move |watch| {
+                if let Some((matcher, _)) = watch {
                     if matcher.is_match(&uev) {
-                        let (_, sender) = w.take().unwrap();
+                        let (_, sender) = watch.take().unwrap();
-                        let _ = sender.send(uev);
+                        let _ = sender.send(uev.clone());
                         return;
                     }
                 }
-            }
+            });
+
             drop(sb); // unlock
         }
     });