Merge "Remove IAccessor implementation and use libbinder APIs" into main
diff --git a/android/virtmgr/src/aidl.rs b/android/virtmgr/src/aidl.rs
index 4b203d6..23652d2 100644
--- a/android/virtmgr/src/aidl.rs
+++ b/android/virtmgr/src/aidl.rs
@@ -33,7 +33,7 @@
     CpuTopology::CpuTopology,
     DiskImage::DiskImage,
     InputDevice::InputDevice,
-    IVirtualMachine::{BnVirtualMachine, IVirtualMachine},
+    IVirtualMachine::{self, BnVirtualMachine},
     IVirtualMachineCallback::IVirtualMachineCallback,
     IVirtualizationService::IVirtualizationService,
     Partition::Partition,
@@ -62,9 +62,8 @@
 use apkverify::{HashAlgorithm, V4Signature};
 use avflog::LogResult;
 use binder::{
-    self, wait_for_interface, BinderFeatures, ExceptionCode, Interface, ParcelFileDescriptor,
-    Status, StatusCode, Strong,
-    IntoBinderResult,
+    self, wait_for_interface, Accessor, BinderFeatures, ConnectionInfo, ExceptionCode, Interface, ParcelFileDescriptor,
+    SpIBinder, Status, StatusCode, Strong, IntoBinderResult,
 };
 use cstr::cstr;
 use glob::glob;
@@ -90,7 +89,7 @@
 use std::sync::{Arc, Mutex, Weak, LazyLock};
 use vbmeta::VbMetaImage;
 use vmconfig::{VmConfig, get_debug_level};
-use vsock::VsockStream;
+use vsock::{VsockAddr, VsockStream};
 use zip::ZipArchive;
 
 /// The unique ID of a VM used (together with a port number) for vsock communication.
@@ -98,6 +97,9 @@
 
 pub const BINDER_SERVICE_IDENTIFIER: &str = "android.system.virtualizationservice";
 
+/// Vsock privileged ports are below this number.
+const VSOCK_PRIV_PORT_MAX: u32 = 1024;
+
 /// The size of zero.img.
 /// Gaps in composite disk images are filled with a shared zero.img.
 const ZERO_FILLER_SIZE: u64 = 4096;
@@ -222,7 +224,7 @@
         console_in_fd: Option<&ParcelFileDescriptor>,
         log_fd: Option<&ParcelFileDescriptor>,
         dump_dt_fd: Option<&ParcelFileDescriptor>,
-    ) -> binder::Result<Strong<dyn IVirtualMachine>> {
+    ) -> binder::Result<Strong<dyn IVirtualMachine::IVirtualMachine>> {
         let mut is_protected = false;
         let ret = self.create_vm_internal(
             config,
@@ -488,7 +490,7 @@
         log_fd: Option<&ParcelFileDescriptor>,
         is_protected: &mut bool,
         dump_dt_fd: Option<&ParcelFileDescriptor>,
-    ) -> binder::Result<Strong<dyn IVirtualMachine>> {
+    ) -> binder::Result<Strong<dyn IVirtualMachine::IVirtualMachine>> {
         let requester_uid = get_calling_uid();
         let requester_debug_pid = get_calling_pid();
 
@@ -1331,14 +1333,14 @@
 }
 
 impl VirtualMachine {
-    fn create(instance: Arc<VmInstance>) -> Strong<dyn IVirtualMachine> {
+    fn create(instance: Arc<VmInstance>) -> Strong<dyn IVirtualMachine::IVirtualMachine> {
         BnVirtualMachine::new_binder(VirtualMachine { instance }, BinderFeatures::default())
     }
 }
 
 impl Interface for VirtualMachine {}
 
-impl IVirtualMachine for VirtualMachine {
+impl IVirtualMachine::IVirtualMachine for VirtualMachine {
     fn getCid(&self) -> binder::Result<i32> {
         // Don't check permission. The owner of the VM might have passed this binder object to
         // others.
@@ -1399,19 +1401,48 @@
 
     fn connectVsock(&self, port: i32) -> binder::Result<ParcelFileDescriptor> {
         if !matches!(&*self.instance.vm_state.lock().unwrap(), VmState::Running { .. }) {
-            return Err(anyhow!("VM is not running")).or_service_specific_exception(-1);
+            return Err(Status::new_service_specific_error_str(
+                IVirtualMachine::ERROR_UNEXPECTED,
+                Some("Virtual Machine is not running"),
+            ));
         }
         let port = port as u32;
-        if port < 1024 {
-            return Err(anyhow!("Can't connect to privileged port {port}"))
-                .or_service_specific_exception(-1);
+        if port < VSOCK_PRIV_PORT_MAX {
+            return Err(Status::new_service_specific_error_str(
+                IVirtualMachine::ERROR_UNEXPECTED,
+                Some("Can't connect to privileged port {port}"),
+            ));
         }
         let stream = VsockStream::connect_with_cid_port(self.instance.cid, port)
             .context("Failed to connect")
-            .or_service_specific_exception(-1)?;
+            .or_service_specific_exception(IVirtualMachine::ERROR_UNEXPECTED)?;
         Ok(vsock_stream_to_pfd(stream))
     }
 
+    fn createAccessorBinder(&self, name: &str, port: i32) -> binder::Result<SpIBinder> {
+        if !matches!(&*self.instance.vm_state.lock().unwrap(), VmState::Running { .. }) {
+            return Err(Status::new_service_specific_error_str(
+                IVirtualMachine::ERROR_UNEXPECTED,
+                Some("Virtual Machine is not running"),
+            ));
+        }
+        let port = port as u32;
+        if port < VSOCK_PRIV_PORT_MAX {
+            return Err(Status::new_service_specific_error_str(
+                IVirtualMachine::ERROR_UNEXPECTED,
+                Some("Can't connect to privileged port {port}"),
+            ));
+        }
+        let cid = self.instance.cid;
+        let get_connection_info =
+            move |_instance: &str| Some(ConnectionInfo::Vsock(VsockAddr::new(cid, port)));
+        let accessor = Accessor::new(name, get_connection_info);
+        accessor
+            .as_binder()
+            .context("The newly created Accessor should always have a binder")
+            .or_service_specific_exception(IVirtualMachine::ERROR_UNEXPECTED)
+    }
+
     fn setHostConsoleName(&self, ptsname: &str) -> binder::Result<()> {
         self.instance.vm_context.global_context.setHostConsoleName(ptsname)
     }
diff --git a/android/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl b/android/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
index afa25e2..e52222a 100644
--- a/android/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
+++ b/android/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachine.aidl
@@ -19,6 +19,13 @@
 import android.system.virtualizationservice.VirtualMachineState;
 
 interface IVirtualMachine {
+    /**
+     * Encountered an unexpected error. This is an implementation detail and the client
+     * can do nothing about it.
+     * This is used as a Service Specific Exception.
+     */
+    const int ERROR_UNEXPECTED = -1;
+
     /** Get the CID allocated to the VM. */
     int getCid();
 
@@ -48,6 +55,19 @@
     /** Open a vsock connection to the CID of the VM on the given port. */
     ParcelFileDescriptor connectVsock(int port);
 
+    /**
+     * Create an Accessor in libbinder that will open a vsock connection
+     * to the CID of the VM on the given port.
+     *
+     * \param instance name of the service that the accessor is responsible for.
+     *        This is the same instance that we expect clients to use when trying
+     *        to get the service with the ServiceManager APIs.
+     *
+     * \return IBinder of the IAccessor on success, or throws a service specific exception
+     *         on error. See the ERROR_* values above.
+     */
+    IBinder createAccessorBinder(String instance, int port);
+
     /** Set the name of the peer end (ptsname) of the host console. */
     void setHostConsoleName(in @utf8InCpp String pathname);
 
diff --git a/tests/vm_accessor/README.md b/tests/vm_accessor/README.md
index c85cf3c..8b0eb2a 100644
--- a/tests/vm_accessor/README.md
+++ b/tests/vm_accessor/README.md
@@ -1,15 +1,16 @@
 # Demo for serving a service in a VM
 
 You can implement a service in a VM, and let client in the Android can use it
-as if it's in the Android. To do so, implement IAccessor.
+as if it's in the Android. To do so, use libbinder's IAccessor.
 
-IAccessor allows AIDL service in a VM can be accessed via servicemanager.
-To do so, VM owners should also provide IAccessor implementation. servicemanager
-will connect to the IAccessor and get the binder of the service in a VM with it.
+IAccessor allows AIDL service in a VM to be accessed via servicemanager.
+To do so, VM owners should also provide IAccessor through libbinder's service
+manager APIs. servicemanager will connect to the IAccessor and get the binder
+of the service in a VM with it.
 
 com.android.virt.accessor_demo apex contains the minimum setup for IAccessor as
 follows:
-  - accessor_demo: Sample implementation of IAccessor, which is expected to
+  - accessor_demo: Sample implementation using IAccessor, which is expected to
       launch VM and returns the Vsock connection of service in the VM.
   - AccessorVmApp: Sample app that conatins VM payload. Provides the actual
       implementation of service in a VM.
diff --git a/tests/vm_accessor/accessor/Android.bp b/tests/vm_accessor/accessor/Android.bp
index 7c0ee6d..8055f91 100644
--- a/tests/vm_accessor/accessor/Android.bp
+++ b/tests/vm_accessor/accessor/Android.bp
@@ -14,7 +14,6 @@
     ],
     rustlibs: [
         "android.system.virtualizationservice-rust",
-        "android.os.accessor-rust",
         "libanyhow",
         "libandroid_logger",
         "libbinder_rs",
diff --git a/tests/vm_accessor/accessor/src/accessor.rs b/tests/vm_accessor/accessor/src/accessor.rs
deleted file mode 100644
index 966bffb..0000000
--- a/tests/vm_accessor/accessor/src/accessor.rs
+++ /dev/null
@@ -1,56 +0,0 @@
-// Copyright 2024, The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//! IAcessor implementation.
-//! TODO: Keep this in proper places, so other pVMs can use this.
-//! TODO: Allows to customize VMs for launching. (e.g. port, ...)
-
-use android_os_accessor::aidl::android::os::IAccessor::IAccessor;
-use binder::{self, Interface, ParcelFileDescriptor};
-use log::info;
-use std::time::Duration;
-use vmclient::VmInstance;
-
-// Note: Do not use LazyServiceGuard here, to make this service and VM are quit
-//       when nobody references it.
-// TODO(b/353492849): Do not use IAccessor directly.
-#[derive(Debug)]
-pub struct Accessor {
-    // Note: we can't simply keep reference by specifying lifetime to Accessor,
-    //       because 'trait Interface' requires 'static.
-    vm: VmInstance,
-    port: i32,
-    instance: String,
-}
-
-impl Accessor {
-    pub fn new(vm: VmInstance, port: i32, instance: &str) -> Self {
-        Self { vm, port, instance: instance.into() }
-    }
-}
-
-impl Interface for Accessor {}
-
-impl IAccessor for Accessor {
-    fn addConnection(&self) -> binder::Result<ParcelFileDescriptor> {
-        self.vm.wait_until_ready(Duration::from_secs(20)).unwrap();
-
-        info!("VM is ready. Connecting to service via port {}", self.port);
-
-        self.vm.vm.connectVsock(self.port)
-    }
-    fn getInstanceName(&self) -> binder::Result<String> {
-        Ok(self.instance.clone())
-    }
-}
diff --git a/tests/vm_accessor/accessor/src/main.rs b/tests/vm_accessor/accessor/src/main.rs
index 49f5794..db53d8e 100644
--- a/tests/vm_accessor/accessor/src/main.rs
+++ b/tests/vm_accessor/accessor/src/main.rs
@@ -14,16 +14,14 @@
 
 //! Android VM control tool.
 
-mod accessor;
 mod run;
 
-use accessor::Accessor;
-use android_os_accessor::aidl::android::os::IAccessor::BnAccessor;
 use anyhow::Error;
 use anyhow::{anyhow, bail};
-use binder::{BinderFeatures, ProcessState};
+use binder::ProcessState;
 use log::info;
 use run::run_vm;
+use std::time::Duration;
 
 // Private contract between IAccessor impl and VM service.
 const PORT: i32 = 5678;
@@ -40,11 +38,13 @@
     );
 
     let vm = run_vm()?;
+    vm.wait_until_ready(Duration::from_secs(20)).unwrap();
+    let accessor = vm.vm.createAccessorBinder(SERVICE_NAME, PORT).unwrap();
+
+    let accessor_delegator = binder::delegate_accessor(SERVICE_NAME, accessor).unwrap();
 
     // If you want to serve multiple services in a VM, then register Accessor impls multiple times.
-    let accessor = Accessor::new(vm, PORT, SERVICE_NAME);
-    let accessor_binder = BnAccessor::new_binder(accessor, BinderFeatures::default());
-    binder::register_lazy_service(SERVICE_NAME, accessor_binder.as_binder()).map_err(|e| {
+    binder::register_lazy_service(SERVICE_NAME, accessor_delegator).map_err(|e| {
         anyhow!("Failed to register lazy service, service={SERVICE_NAME}, err={e:?}",)
     })?;
     info!("service {SERVICE_NAME} is registered as lazy service");